12rcu commented on PR #5158: URL: https://github.com/apache/ignite-3/pull/5158#issuecomment-2658732593
> Hello @12rcu , thank you for the PR. I have a couple of questions: > > > I changed the test annotation to a retrying test so that the test can fail once and still pass. > > Why would we need that? If the test is flaky, it needs to be fixed, we don't want to hide potential problems. > > > One source of flakiness during testing was the election timeout > > Could you please explain the exact reasons why this affects the flakiness? I didn't understand the explanation provided in your description. > > > This PR is not intended to fix the flakiness of this test per se, but to mitigate the timeout issues > > What timeout issues are you talking about? The retry annotation: I also don't like this annotation for the same reason, but I thought we might consider it since the timeouts for the `waitForCondition()` method are fixed and don't account for system load. So if the machine has a high load and fails because it ran into the timeout, it has another chance to pass the test. I would leave it up to you, I would remove it. The election timeout flakyness: When the election timeout is triggered, a new leader is elected and all nodes stop following and refollow a new leader, incrementing the `getOnStartFollowingTimes()` and `getOnStopFollowingTimes()`. Since this was unexpected, the assertions after that fail. What timeout issues are you talking about? The election timeout and `waitForCondition()` timeout -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
