Skip to content
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

Reduce wait period to avoid flakiness #8701

Merged
merged 10 commits into from
Oct 11, 2024
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ jobs:
path: build/test-results

acceptanceTests:
parallelism: 5
parallelism: 4
executor: machine_large_executor_amd64
steps:
- install_java_21
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public class MultiPeersStandAloneVcBlocksAcceptanceTest

firstTekuNode.start();

firstTekuNode.waitForEpochAtOrAbove(2);
firstTekuNode.waitForEpochAtOrAbove(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're changing this anyway, i'd change this back to 2, and i'd change the genesis wait time to 60 seconds to let both nodes be online close to genesis...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but i'd also be starting node 2 prior to waiting for epoch 2 i think...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wouldn't work because we would have to wait until epoch 2 to start the second node in the current logic. This would only drag the test length to a longer period of waiting. What we could do is possibly change the slashing to be in a block for the epoch 4 or 5.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the BN will come up slow, so if the second BN starts before genesis and BOTH BN wait for the 2 epochs, then the second VC starts, by then the world order is a lot less ugly i think you'll find.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't think we should increase the lenght of genesis from 10 to 60 and make this longer than it needs to be, test usually succeeds in 50 secs. So making it longer for no reason doesn't sound a better solution. I'm ok with starting beacon2 earlier.


final int slashedValidatorIndex = 34;
final BLSKeyPair slashedValidatorKeyPair = getBlsKeyPair(slashedValidatorIndex);
Expand All @@ -74,7 +74,6 @@ public class MultiPeersStandAloneVcBlocksAcceptanceTest
.withGenesisTime(genesisTime)
.withNetwork(network)
.withRealNetwork()
.withRealNetwork()
.withAltairEpoch(altairEpoch)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is removal of withAltairEpoch intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think so, although this was added by Paul's commit, we never actually reached the epoch where Altair was set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it was pointless to have altair at epoch 100 when we're finishing in a few epochs...

.withPeers(firstTekuNode)
.build());
Expand Down