New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ignite-6430: CacheGroupsMetricsRebalanceTest.testRebalanceEstimateFinishTime test fails periodically #3914
Conversation
|
||
try { | ||
while (finishRebalanceLatch.getCount() != 0) { | ||
while (finishRebalanceLatch.getCount() != 0L) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove L postfix from this place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan Done
@@ -223,26 +223,26 @@ public void testRebalanceEstimateFinishTime() throws Exception { | |||
// Waiting 25% keys will be rebalanced. | |||
int partKeys = KEYS / 2; | |||
|
|||
final long keysLine = (long)(partKeys - (partKeys * 0.25)); | |||
final long keysLine = partKeys - partKeys / 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, it could be transform to something like partKeys*3/4, due to partKeys is long and partKeys is int and overflow is excluded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan Done
|
||
long timePassed = U.currentTimeMillis() - startTime; | ||
long timeLeft = finishTime - System.currentTimeMillis(); | ||
currentTime = System.currentTimeMillis(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be use U.currentTimeMillis() instead of System.currentTimeMillis()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan Done
); | ||
|
||
System.clearProperty(IGNITE_REBALANCE_STATISTICS_TIME_INTERVAL); | ||
|
||
System.out.println("Rebalance time:" + (U.currentTimeMillis() - startTime)); | ||
currentTime = System.currentTimeMillis(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about U.currentTimeMillis()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan Done
|
||
assertTrue(finishRebalanceLatch.await(timeLeft + 2_000, TimeUnit.SECONDS)); | ||
assertTrue("Got timeout while waiting to rebalance. Estimated left time: " + timeLeft + '.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Got timeout while waiting for rebalance'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan Done
assertTrue((U.currentTimeMillis() - startTime) < 5000); | ||
assertTrue((U.currentTimeMillis() - startTime) > 0); | ||
assertTrue("Invalid start time: " + startTime + ", but the current time: " + currentTime + '.', | ||
startTime > 0L && (currentTime - startTime) >= 0L && (currentTime - startTime) <= 5000L); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should take rebalancing delay into consideration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan There is no delay in CACHE1
config.
|
||
assertTrue("Expected less 5000, Actual:" + diff, Math.abs(diff) < 10_000); | ||
assertTrue("Expected less than 5000, but actual: " + diff + '.', Math.abs(diff) < 10_000L); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected less than 10000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan Done
@@ -254,32 +254,44 @@ public void testRebalanceEstimateFinishTime() throws Exception { | |||
} | |||
}); | |||
|
|||
assertTrue(latch.await(getTestTimeout(), TimeUnit.MILLISECONDS)); | |||
assertTrue("Got timeout while waiting 25% keys will be rebalanced.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this message, pls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan Done
|
||
CacheMetrics metrics = ig2.cache(CACHE1).localMetrics(); | ||
|
||
long startTime = metrics.getRebalancingStartTime(); | ||
long currentTime = System.currentTimeMillis(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change it with U.currentTimeMillis(); pls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@1vanan Done
605b070
to
6275b6b
Compare
For IGNITE-6430
Please see my comments in Ignite Jira if you a bit lost in PR.