tests: close open file descriptors from make full node#5057
tests: close open file descriptors from make full node#5057algonautshant wants to merge 5 commits intoalgorand:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5057 +/- ##
==========================================
+ Coverage 53.56% 53.57% +0.01%
==========================================
Files 430 430
Lines 54091 54096 +5
==========================================
+ Hits 28975 28984 +9
+ Misses 22869 22867 -2
+ Partials 2247 2245 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| return err | ||
| } | ||
| } | ||
| part.Close() |
There was a problem hiding this comment.
this is probably not a right place since there are lots of return statement within the if block above.
I suggest to wrap the if block in a function and defer closing like this:
if err != nil {
func() {
defer part.Close()
handle.Close()
...
}()
}
There was a problem hiding this comment.
part is used when deleting part keys. So, closing here is not right.
| } | ||
| } | ||
|
|
||
| func (node *AlgorandFullNode) Shutdown() { |
There was a problem hiding this comment.
how is it different from Stop() ? mayb move these close statements there?
There was a problem hiding this comment.
registry, err := ensureParticipationDB(genesisDir, node.log)
opened in MakeFull and not reopened in Start, so closing in Shutdown:
node.accountManager.Registry().Close()`
Same for
crashAccess, err := db.MakeAccessor(crashPathname, false, false)
opened in MakeFull and not reopened in Start, so closing in Shutdown:
node.agreementService.Accessor.Close()
There was a problem hiding this comment.
then Shutdown should be called in daemon/algod/server.go's Server::Stop
|
please rebase on top of master |
algonautshant
changed the title
* node_test runs multiple nodes in a single process that leads to file descriptors leak (see algorand#5057 for more details). * just closing ledger is not enough because of concurrent operations evaluation operations done by transaction pool. * made transaction pool shutdown-able and stop it before ledger termination.
|
Pulled some changes into #6039 - with new p2p tests in node_test this became an issue. |
3 participants
The tests using MakeFull node leave behind many many open files.
This PR closes these.
Some are closed as part of the test procedure, others needed changes to the node.go code.
The node has Start and Stop procedures, but some file descriptors are opened before Start, and not closed at Atop.
For these, Shutdown procedure is introduced to close them.
Despite these improvements, the test still fails occasionally. The reason I think is the network connections not getting stablished. So, this tests are still flaky, and should not be enabled yet.