RemoveUntraceableHeaders #3750
Merged
RemoveUntraceableHeaders #3750
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roman-khimov
added
I3
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3750 +/- ##
==========================================
+ Coverage 82.83% 82.98% +0.15%
==========================================
Files 335 335
Lines 46850 46782 -68
==========================================
+ Hits 38806 38821 +15
+ Misses 6450 6367 -83
Partials 1594 1594 ☔ View full report in Codecov by Sentry. |
AnnaShaleva
reviewed
Dec 13, 2024
There was a problem hiding this comment.
It's too easy to forget about some part of the code that uses headers. We need to test:
- Chain state reset and further node synchronisation, just ensure that it goes normally. I didn't find anything in the code, but may be we missed something.
- Chain work with GC on/off, StateRootInheader on/off and
KeepOnlyLatestStateconfig on/off. Check that headers are removed properly, check that the node can be restarted and the rest of services work correctly. - Ideally, P2P state exchange extension, but right now it's possible to test only with NeoFSBlockFetcher off (Implement headers fetching via NeoFS BlockFetcher service #3574).
AnnaShaleva
reviewed
Dec 13, 2024
With blocks available from NeoFS we can drop them from the local DB. Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Roman Khimov <roman@nspcc.ru>
It's there since 423c788, but looks like it never changed anything, the same thing is done six lines above and tgtBlock is not changed since. Signed-off-by: Roman Khimov <roman@nspcc.ru>
Transfer data is timestamp-based, previously it always had and used headers,
no we can go via a small cache (we don't want it to grow or be stored forever).
Otherwise it's unable to do the job:
2024-12-13T12:55:15.056+0300 ERROR failed to find block header for transfer GC {"time": "19.066µs", "error": "key not found"}
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Be a nice neighbor, try to reply with whatever headers we have, don't fail completely because of a single missing header. Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov
force-pushed
the
removeuntraceableheaders
branch
from
642416d to
90b6a42
Compare
AnnaShaleva
approved these changes
Dec 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Keeping headers we no longer need.
Solution
Remove them along with blocks.