aeternity / elixir-node

Elixir full node implementation of the aeternity specification
ISC License
212 stars 38 forks source link

[GH-733] Implemented garbage collection function for transactions pool #794

Closed DanielaIvanova closed 5 years ago

DanielaIvanova commented 5 years ago

resolves #733 @cytadela8 We are having some problems with multi node sync test. Could you help us?

thepiwo commented 5 years ago

@grzegorz225 could you help with the failing tests eventually?

cytadela8 commented 5 years ago

@DanielaIvanova The problem is you call Pool.garbage_collection inside Chain.handle_call(:add_validated_block, but Pool.garbage_collection calls Chain.top_height. Chain.top_height can't finish till the previous call finishes, but the previous call can't finish till Pool.garbage_collection finishes...

cytadela8 commented 5 years ago

I didn't trigger the bug when testing your code for some reason, but this should be it.

cytadela8 commented 5 years ago

To fix this you can change Pool.garbage_collect into a cast so it will be executed in parallel and won't block Chain.handle_call(:add_validated_block

DanielaIvanova commented 5 years ago

@cytadela8 We call Pool.garbage_collection(height) inside Chain.handle_call(:add_validated_block) and we are passing the height of the incoming block, not Chain.top_height(). We switched to handle_cast but the problem still persists.

DanielaIvanova commented 5 years ago

We tested with 3 synced nodes.

  1. Synced all of them
  2. Node2 added transaction to the pool with ttl = 4 (Node2 doesn't have money)
  3. Node1 added transaction to the pool as well, with ttl = 4 (Node3 doesn't have money)
  4. Node0 is the miner. Those transactions persist in the pool(currently height = 0).

On height = 4 the transactions are still in pool. On height = 5 the pool is empty.

This manual test proves that the garbage collector works as intended with synced nodes as well. We don't know where the problem is coming from.