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.

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

Release can deadlock if node cache is too small #694

Closed
HerbertJordan opened this issue Dec 14, 2023 · 3 comments
Closed

Release can deadlock if node cache is too small #694

HerbertJordan opened this issue Dec 14, 2023 · 3 comments
Assignees
Labels
bug Something isn't working s5 scheme 5 - MPT workaround exists

Comments

@HerbertJordan
Copy link
Collaborator

This issue was encountered in a CI test testing the releasing of accounts with large states. The recursive release operation may keep write locks on nodes that get scheduled to be flushed.

A temporary fix was the increase of the node cache. A future improvement may perform the release iteratively, not holding locks on nodes for long durations.

@HerbertJordan HerbertJordan added the bug Something isn't working label Dec 14, 2023
@HerbertJordan HerbertJordan self-assigned this Mar 19, 2024
@kjezek kjezek added s5 scheme 5 - MPT workaround exists labels Apr 3, 2024
@HerbertJordan
Copy link
Collaborator Author

The number of locks held by the asynchronous node release worker should be limited to the maximum trie height since it utilises a depth-first iteration through the trie that is released. Furthermore, any node that is accesses during the release is accessed through the getWriteAccess function, which causes the accessed node to be at the begin of the node cache, far way from the end where nodes are flushed and released.

Thus, given that tries are at most 30-40 nodes high, and caches contain several thousands of nodes, nodes to be flushed should not bot be blocked by the background node releaser.

Ultimately, however, this is related to #686 describing that the node cache is an inherent limitation on the working set, which includes the set of nodes accessed during releases.

@HerbertJordan
Copy link
Collaborator Author

With #780 stress tests for deleting large accounts have been added, and so far no more issues for those situations have been identified in nightly builds.

@HerbertJordan
Copy link
Collaborator Author

I am closing this; obsolete;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working s5 scheme 5 - MPT workaround exists
Projects
None yet
Development

No branches or pull requests

2 participants