benchmark: forcefully close processes#43557
Conversation
|
CC: @nodejs/build |
nodejs-github-bot
added
the
benchmark
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
I am fine to do this but I fear we might hide an actual bug doing it. Even though that's not really critical as it's only in our own benchmark suite.
Should we maybe increase the timeout to e.g., 5 seconds? The machines could be under heavy load.
FWIW the benchmark machines in the CI are baremetal machines (donated by Intel for the purposes of benchmark work). The only load on the machines should be Jenkins and the code excuted by the benchmark job. |
|
Yes, I tested it, it's not a load problem. |
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
I guess it does not hurt to do this.
ShogunPanda
force-pushed
the
force-close-benchmarks
branch
from
ca74cb6 to
fe5dc88
Compare
PR-URL: #43557 Fixes: nodejs/build#2968 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
Landed in 684e107 |
PR-URL: #43557 Fixes: nodejs/build#2968 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: #43557 Fixes: nodejs/build#2968 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: #43557 Fixes: nodejs/build#2968 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs/node#43557 Fixes: nodejs/build#2968 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
6 participants
This PR makes sure a benchmark always exit after reporting results.
The timer is created in order to allow the process to exit normally.
This solves situations in which some resources are wrongfully reported as active (like HTTP sockets closed in a bad way by wrk) and prevent the benchmarks to continue.
Fixes: nodejs/build#2968