test: storage segment module unit tests#259
Merged
xdustinface merged 1 commit intorefactor/storage_segments_cleanupfrom Dec 10, 2025
Merged
test: storage segment module unit tests#259xdustinface merged 1 commit intorefactor/storage_segments_cleanupfrom
xdustinface merged 1 commit intorefactor/storage_segments_cleanupfrom
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
xdustinface
approved these changes
Dec 10, 2025
xdustinface
merged commit 4301d3e
into
refactor/storage_segments_cleanup
24 checks passed
ZocoLini
added a commit
that referenced
this pull request
Dec 10, 2025
ZocoLini
added a commit
that referenced
this pull request
Dec 11, 2025
xdustinface
pushed a commit
that referenced
this pull request
Dec 12, 2025
* segments cache struct and evict method generalized * save segments to disk made generic inside the segments cache struct * save_dirty_segments logic refactorized * tbh I dont know waht I refactored here * removed io module inside storage/disk * you are right rabbit * store in segment moved to SegmentCache * sentinel headers creation moved to the Persistable trait and encapsulated there * unified sentinel item behaviour - no longer compiles bcs the tip_high calculation * renames * new struct to manage the hashmap of segments and tip height moved - doesnt compile yet, wip * get_headers generalized in the SegmentsCache struct - wip, not compiles * store headers logic moved to the SegmentsCache and introduced method to better handle tip_height and storage index - wip, no compiles * store_headers_impl using precomputed_hashes didn't provide real benefics with the current usage - wip, no compiles * removed unused StorageManager::get_headers_batch methos - wip, no compiles * removed warnings * ensure segment loaded moved inside the SegmentsCache with a logic change, we ask for a segment and if it doesn't exist in memory we load it from disk - wip, no compiles * const MAX_ACTIVE_SEGMENTS encapsulated - wip, no compiles * removed one commit as it is fixed * created a SegmentsCache::store_headers_at_height - wip, no compiles * removed inconsistency when loading segments metadata * removed to methods unused bcs now that behaviour is encapsulated * building SegmentCache yip_height when creating the struct * removed unused function * some refactor and removed the notification enum and related behaviour - wip, no compiles * disk storage manager worker no longer needs cloned headers to work * renamed segments stoage fields * removed new unused function * evict logic removed * save dirty segments logic moved into SegmentsCache * clippy warnings fixed * save dirty is now an instance method for the DiskStorageManager * when persisting segments we try to create the parent dirs to ensure they exist * improved logic to ensure different clear behaviour for SegmentsCache * correctly rebuilding the block reverse index * fixed bug found by test test_checkpoint_storage_indexing * fixed bug updating tip_height in SegmentCache thanks spotted by test test_filter_header_segments * fixed bug, we stop persisting segments after we find the first sentinel, to correctly initialize valid_count - bug spotted by test test_filter_header_persistence * refactor: HEADER_PER_SEGMENT encapsulated inside segment and renamed to ITEMS_PER_SEGMENT - wip, no compiles * block index rebuild logic moved into SegmentCache<BlockHeader> and load_segment_metadata renamed in flavor of a better name for its current behaviour being the block index contructor * added some cool inlines * fixed test that was creating a centinel filter header at height 0 making the segment not persist entirely * renamed header reference to item in segments.rs so its clear that the new struct can work with any struct * clippy warning fixed * logging when storing multiple items simplified * removed sentinel headers from the segments logic * unit tests for the segment and segment_cache structs after the refactor (#259) * removed unused methods after rebase * renamed and removed old documentation for store_headers_impl * refactorized and adjusted docs for conversion methods between stoage index, height and offset * removed old comments and forcing to give sync_base_height when creating the SegmentCache * quick fix to load sync_base_height if persisted before * review comments addressed * read block index operation made async * using atomic write where we write to disk
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
With the new tests introduced by this PR and the already existing in the codebase we archive a 88.84% (382/430) line coverage.
The lines that are not being tested are the corresponding to
build_block_index_from_segments. I propose waiting until we have a BlockIndex struct to manage the block index logic and test this behaviour in it's unit testsTests #244