Skip to content

Conversation

roberto-bayardo
Copy link
Collaborator

@roberto-bayardo roberto-bayardo commented Jun 24, 2025

  • Dropping a cursor in a middle of an iteration was resulting in dangling items. This PR will re-attach any dangling items in drop(). Unfortunately this means every cursor opened against an entry must end up traversing the entire list, even if the desired item is found right away. It also changes how items end up ordered after each cursor lifecycle.

  • Also makes every Index test have a "test_index" prefix.

@roberto-bayardo roberto-bayardo force-pushed the fix-index-cursor-update branch 3 times, most recently from ef249ae to 902b464 Compare June 24, 2025 22:52
@roberto-bayardo roberto-bayardo changed the title [storage/index] unit tests demonstrating cursor update bug [storage/index] fix + unit test for cursor update bug Jun 24, 2025
@roberto-bayardo roberto-bayardo force-pushed the fix-index-cursor-update branch from 902b464 to 9e46d21 Compare June 24, 2025 22:59
@roberto-bayardo roberto-bayardo marked this pull request as ready for review June 24, 2025 23:02
Copy link
Collaborator

@dnkolegov-ar dnkolegov-ar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The fuzz test after an hour could not reproduce the bug.

@patrick-ogrady
Copy link
Contributor

patrick-ogrady commented Jun 25, 2025

Replacing with #1155

Copy link

codecov bot commented Jun 25, 2025

Codecov Report

Attention: Patch coverage is 56.00000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 91.01%. Comparing base (fddae18) to head (9e46d21).

Files with missing lines Patch % Lines
storage/src/index/storage.rs 56.00% 11 Missing ⚠️
@@            Coverage Diff             @@
##             main    #1152      +/-   ##
==========================================
- Coverage   91.03%   91.01%   -0.02%     
==========================================
  Files         215      215              
  Lines       57425    57440      +15     
==========================================
+ Hits        52276    52281       +5     
- Misses       5149     5159      +10     
Files with missing lines Coverage Δ
storage/src/index/mod.rs 87.50% <ø> (ø)
storage/src/index/storage.rs 95.72% <56.00%> (-3.59%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fddae18...9e46d21. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@BrendanChou BrendanChou deleted the fix-index-cursor-update branch September 8, 2025 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants