Skip to content

[SDSTOR-23023] Add option to commit_blk for post-crash resync#896

Merged
koujl merged 1 commit into
eBay:stable/v7.xfrom
koujl:recommit
Jul 15, 2026
Merged

[SDSTOR-23023] Add option to commit_blk for post-crash resync#896
koujl merged 1 commit into
eBay:stable/v7.xfrom
koujl:recommit

Conversation

@koujl

@koujl koujl commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

https://jirap.corp.ebay.com/browse/SDSTOR-23023

Normally commit_blk is called either during recovery (which advances both watermarks) or after a fresh alloc. However, in some scenarios, a blk_id may be recorded to be live by the caller but the allocator watermarks have not been successfully persisted.

The new force_cache parameter allows the caller to unconditionally advance both m_last_append_offset and m_commit_offset, covering such cases without requiring the system to be in initialising mode.

Related PR: eBay/HomeObject#452

@codecov-commenter

codecov-commenter commented Jul 13, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (stable/v7.x@20df560). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/lib/blkdata_svc/blkdata_service.cpp 33.33% 1 Missing and 1 partial ⚠️
src/lib/device/virtual_dev.cpp 66.66% 0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@              Coverage Diff               @@
##             stable/v7.x     #896   +/-   ##
==============================================
  Coverage               ?   48.18%           
==============================================
  Files                  ?      110           
  Lines                  ?    12964           
  Branches               ?     6229           
==============================================
  Hits                   ?     6247           
  Misses                 ?     2581           
  Partials               ?     4136           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

Besroy
Besroy previously approved these changes Jul 14, 2026

@Besroy Besroy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix

JacksonYao287
JacksonYao287 previously approved these changes Jul 15, 2026

@xiaoxichen xiaoxichen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the force_cache is a bit wired, maybe force is enough or pick a better name (the cache expose internal implementation and wired to api user)

@xiaoxichen

Copy link
Copy Markdown
Collaborator

other part LGTM

Normally commit_blk is called either during recovery (which advances
both watermarks) or after a fresh alloc. However, in some scenarios, a
blk_id may be recorded to be live by the caller but the allocator
watermarks have not been successfully persisted. The new `recommit`
parameter allows the caller to unconditionally advance both
m_last_append_offset and m_commit_offset, covering such cases without
requiring the system to be in initialising mode.

Signed-off-by: Kou, Jilong <jkou@ebay.com>
@koujl
koujl dismissed stale reviews from JacksonYao287 and Besroy via 82997cd July 15, 2026 06:20
@koujl koujl changed the title [SDSTOR-23023] Add force_cache option to commit_blk for post-crash resync [SDSTOR-23023] Add option to commit_blk for post-crash resync Jul 15, 2026
@koujl

koujl commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

the force_cache is a bit wired, maybe force is enough or pick a better name (the cache expose internal implementation and wired to api user)

Good idea, changed to recommit which better captures our intent (re-commit a allocated blkid). Updated the argument description accordingly.

@koujl
koujl merged commit 7eafb3a into eBay:stable/v7.x Jul 15, 2026
41 of 42 checks passed
@koujl
koujl deleted the recommit branch July 15, 2026 08:31
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.

5 participants