phoreproject / graphene

Phore Synapse working repository
MIT License
13 stars 6 forks source link

stateDerivedFromBlock.lastSlotReceipts uses unreasonable too much RAM #108

Open wqking opened 4 years ago

wqking commented 4 years ago

In a simple test (not many slots, maybe just one or two days), a lastSlotReceipts can hold more than 5M receipts, which is more than 100M RAM. And if there are more slots, I ever observed it uses too much RAM and crashes the OS.

I'm not sure if it can be eliminated, if not, we may need to use external database for it instead of residenting in memory. If we are going to use database, I can do it. For the choice of database, current database for the chain uses very much disk. I think SQLite maybe better. The disadvantage of using SQLite is that we introduce a new kind of database to Synapse.

meyer9 commented 4 years ago

It shouldn't ever need to calculate state that far in the future. Where is this being called from? Maybe we should set some limits on the derived state (5 epochs or so).

wqking commented 4 years ago

StateManager.GetStateForHashAtSlot calls stateDerivedFromBlock.deriveState which populates the receipts array. GetStateForHashAtSlot is called when adding a block to the statemap (AddBlockToStateMap).

meyer9 commented 4 years ago

So I guess we should reject blocks too far from the current tip so we don't DoS the node when trying to calculate the state at that time?

wqking commented 4 years ago

Currently the Receipt list are only used for debug log and SyncManager.postProcessHook, so maybe, 1, if the receipts are only used for that purpose, we may remove them from memory after they are used by the hook. 2, if the receipts are used for other purpose, we may store it to local database, then load them on demand. 3, or as you said, we may reduce the generating of the receipts.

meyer9 commented 4 years ago

Let's reduce receipt generation. In ProcessBlock, ensure the block being processed is within 3 epochs of the parent block. In GetEpochInformation, make sure we're not getting a block more than 3 epochs from the current tip slot.

wqking commented 4 years ago

OK, I will do it.

wqking commented 4 years ago

After some research, I doubt limiting block age can work, because no matter the blocks are old or new, they will be processed eventually.

What I've found is, after Beacon is shut down for some time, and after it runs again, the function primitives.State.ProcessSlots will try to make the slot catching up to latest slot, so it calls ProcessEpochTransition many times on each epoch, which will generate huge amount of receipts. That also means the longer the Beacon was shut down, the more receipts it will generate after run. I think we may need to limit the number of slots (or epoches) that ProcessSlots can process to generate the receipts?

Another problem (maybe not a problem, just how it should work) is, there are 256 validators in the .json config, but I only run one validator. Beacon still considers all 256 validators are 'Active' and generate receipts for them. I guess it's how Beacon works, it requires the validators running, otherwise give penalty to them?

meyer9 commented 4 years ago

It does that when the validation needs them to create new blocks. That's why limiting the age will work because the validator will stop until we sync to a block closer to the current time.

wqking commented 4 years ago

I tried a simple fix here on 7d1b61d2b9319c969ca43870714e359e95d7c4d7 Please reivew it. I didn't observe those limitations were triggered so the memory usage is not decreased. Also, when Beacon starts up, when it loads blocks from database and populates the state map, it will generate large amount of receipts.

meyer9 commented 4 years ago

Where is it calling derive state from that generates a ton of receipts? It shouldn't have to derive any state when loading. It will have to derive state if a validator connects and requests epoch info after being offline for a while.

I did notice you have the wrong inequality on GetEpochInformation

wqking commented 4 years ago

The function populateStateMap in file beacon.database calls Blockchain.AddBlockToStateMap, which will populate the receipts. That's from my memory, I will test to check it.

meyer9 commented 4 years ago

That should only need to derive state between blocks I think though.

If Block A has slot 10 and block B has slot 20 and A is B's parent, it should derive state between 10 and 20 which makes sense.