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.

Sign up for GitHub

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

Jump to bottom

Improve stability of CI tests #878

Merged
merged 50 commits into from
Jun 1, 2020
Merged

Improve stability of CI tests #878

merged 50 commits into from
Jun 1, 2020

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented May 8, 2020 •
edited
Loading

  • Use non-blocking randomness for acceptance tests
  • wait 30 seconds instead of 2 when killing AT processes
  • use java scheduled executor timers instead of vert.x (everywhere, not just ATs).

Signed-off-by: Danno Ferrin danno.ferrin@gmail.com

@shemnon
Improve stability of CI tests
8086639 
* Set default nat mode to NONE instead of AUTO
* Ignore the flakey rocksdb unit test.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon shemnon mentioned this pull request May 8, 2020
Closed
@shemnon
debug logging
5f5ef74 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
adjust log level in circleci config instead
d842135 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
* bump circleci to large
cddc48f 
* dump memory before shutting down cluster

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
undo accidental commit inclusion
34df609 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
* Reduce table refresh interval for acceptance tests from 30->5 sec
ef629ce 
* plumb option to CLI via -X command

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
Merge branch 'master' of github.com:hyperledger/besu into circleci
960504d 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
fix build breakages
f03ee75 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
spotless
7af3e8c 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
don't have bootnodes refresh as often, make refresh just past round time
719c40a 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@RatanRSur RatanRSur added the flake 60% of the time it works 100% of the time. label May 8, 2020
@shemnon
move where we set refresh
f9f11b3 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
update jdk versions
3d2cac9 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
wrong toy story character
8dbf9c5 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
libsodium update
e2f8c5a 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
prefer endpoint port over udp port.
baa17aa 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
fix units error
0dcb736 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
spotless
6404c59 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
take vertx down to 0
9a1bb46 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@RatanRSur RatanRSur assigned RatanRSur and unassigned RatanRSur May 11, 2020
@timbeiko timbeiko added this to the Chupacabra Sprint 65 milestone May 20, 2020
@shemnon
Acceptance Tests non-blocking PRNG
ab821e3 
In both the threaded nodes and the harness set the entroy gathering
device URL to empty string so the default SecureRandom will be DRBG
using non-blocking randomness.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
Merge branch 'acceptanceNoNativePRNG' of github.com:shemnon/besu into…
67a0536 
… circleci

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
log cluster completed
dc8a9da 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
noisy secure random stats
22b8684 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
undo ignore test
365c4d1 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
log discovery packets sent
31b847e 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
make securerandom provider quieter.
25f62b1 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
shemnon added 12 commits May 27, 2020 11:24
@shemnon
more devp2p logging
84e466b 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
spotless
94bdbb1 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
not all packets are pings...
45af203 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
lotso tracing
26348f3 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
log timer calls
1f46d89 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
who put that really annoying errorprone check in?
463068c 
Oh, yea.  That was me.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
another log.
bcd7904 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
and log stacktrace
9c6a3f2 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
vertx.io rollback
a9c616d 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
make it fail
91cd6ba 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
make it fail
ce9e619 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
fine, we'll make our own timers
8777ab1 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
remove excessive debugging
574eaee 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
undo code cleanup, so that elsewhere
77e6224 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
mbaxter
mbaxter reviewed May 28, 2020
View reviewed changes
...c/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/IndirectVertxTimerUtil.java Outdated
timers.put(
id,
secheduledExecutor.scheduleAtFixedRate(
() -> vertx.executeBlocking(e -> handler.handle(), r -> {}),
Copy link
Contributor

Choose a reason for hiding this comment

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

One issue to investigate here is whether this will cause threading issues. IIRC, VertexTimerUtil guarantees the timed callbacks happen in the calling thread, and I think we need that as the code is currently written ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be the event loop or io thread in this case. Doing cryptography in the event loop or io thread could be a limiter to the number of concurrent peers we can service, especially when it could block on randomness.

While I haven't proven it yet I think this (too much processing, like walking a 4000 entry hash table) is what's causing our eth/65 issues, so getting stuff off of the io thread looks to be a long term goal.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense. We just need to make sure that the code we're executing with this timer is thread-safe.

@shemnon
fix tpyos
0782b64 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
Merge branch 'master' into circleci
028ea77
@shemnon
undo indirect timer
9f44951 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
Merge branch 'master' into circleci
5b6c644
RatanRSur
RatanRSur approved these changes Jun 1, 2020
View reviewed changes
Copy link
Contributor

@RatanRSur RatanRSur left a comment

Choose a reason for hiding this comment

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

flakiness-master failures: 118/25 avg:4.72
refs/pull/878/head failures: 36/14 avg:2.5714285714285716

The burn-in testing on this looks good. When you drill into the logs I attached, you'll see that there's only one kind of failure in this branch and it's due to something @shemnon said he can roll back.
878-burn-in-results.zip

@shemnon
ignore newly flakey test
7d2a812 
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
Copy link
Contributor Author

shemnon commented Jun 1, 2020 •
edited
Loading

Because it's just a single test that is consistently flakey I @Ignored it and posted #1011 to fix it.

@RatanRSur
Copy link
Contributor

Ok, in that case let's add the "fixes ..." for all the flake labels so they can easily be found in the future.

https://github.com/hyperledger/besu/issues?q=is%3Aopen+is%3Aissue+label%3Aflake

@shemnon shemnon merged commit dac36a5 into hyperledger:master Jun 1, 2020
macfarla pushed a commit to macfarla/besu that referenced this pull request Jun 3, 2020
@shemnon @macfarla
Improve stability of CI tests (hyperledger#878)
16458d7 
* Use non-blocking randomness for acceptance tests
  This addresses entropy draining during unit tests.
* wait 30 seconds instead of 2 when killing AT processes
* mark NodeSmartContractPermissioningIbftStallAcceptanceTest as @ignore since it has become reliably and specifically flakey.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
This was referenced Jun 3, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbaxter mbaxter mbaxter left review comments

@lucassaldanha lucassaldanha lucassaldanha left review comments

@RatanRSur RatanRSur RatanRSur approved these changes

Assignees
No one assigned
Labels
flake 60% of the time it works 100% of the time.
Projects
None yet
Milestone
Chupacabra Sprint 65
Development

Successfully merging this pull request may close these issues.
5 participants
@shemnon @RatanRSur @mbaxter @lucassaldanha @timbeiko