Skip to content

crimson/os/seastore: prevent duplicate journal batch flushes - #73072

Open
myoungwon wants to merge 1 commit into
ceph:mainfrom
myoungwon:wip-fix-wait_unfull_flush_promise
Open

myoungwon wants to merge 1 commit into
ceph:mainfrom
myoungwon:wip-fix-wait_unfull_flush_promise

Conversation

@myoungwon

Copy link
Copy Markdown
Member

When the journal I/O depth reaches FULL, RecordSubmitter waits for an outstanding I/O to complete before flushing the pending batch.

When an outstanding I/O completes, decrement_io_with_flush() is called to release an I/O slot and flush the pending batch if RecordSubmitter was FULL.
In decrement_io_with_flush(), flush_current_batch() is called before wait_unfull_flush_promise is resolved and reset. Note that flushing the batch may trigger another I/O completion callback, re-entering decrement_io_with_flush() before the promise is reset.

Since wait_unfull_flush_promise is still present, the reentrant call attempts to flush the current batch again, even though the first flush has already submitted its records and replaced the current batch with an empty one.

This can result in encode_records() being called with an empty record group, causing the metadata length assertion to fail:

ceph_assert(bl.length() == record_group.size.get_raw_mdlength());

To prevent this, move wait_unfull_flush_promise to a local variable and reset the optional before calling flush_current_batch().

Signed-off-by: Myoungwon Oh ohmyoungwon@gmail.com

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

When the journal I/O depth reaches FULL, RecordSubmitter waits for
an outstanding I/O to complete before flushing the pending batch.

When an outstanding I/O completes, decrement_io_with_flush() is
called to release an I/O slot and flush the pending batch if
RecordSubmitter was FULL.
In decrement_io_with_flush(), flush_current_batch() is called
before wait_unfull_flush_promise is resolved and reset.
Note that flushing the batch may trigger another I/O completion callback,
re-entering decrement_io_with_flush() before the promise is reset.

Since wait_unfull_flush_promise is still present, the reentrant
call attempts to flush the current batch again, even though the
first flush has already submitted its records and replaced the
current batch with an empty one.

This can result in encode_records() being called with an empty
record group, causing the metadata length assertion to fail:

ceph_assert(bl.length() == record_group.size.get_raw_mdlength());

To prevent this, move wait_unfull_flush_promise to a local variable
and reset the optional before calling flush_current_batch().

Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>
@myoungwon
myoungwon requested a review from a team as a code owner October 25, 2026 11:54
@Matan-B Matan-B added this to Crimson Oct 25, 2026
@Matan-B Matan-B moved this to Needs QA in Crimson Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Needs QA

Development

Successfully merging this pull request may close these issues.

2 participants