Skip to content

fsync Implementation#23

Draft
vidyasilai wants to merge 2 commits into
mathworks:mainfrom
vidyasilai:fsync_impl
Draft

fsync Implementation#23
vidyasilai wants to merge 2 commits into
mathworks:mainfrom
vidyasilai:fsync_impl

Conversation

@vidyasilai
Copy link
Copy Markdown
Collaborator

No description provided.

@vidyasilai vidyasilai requested a review from tonyastolfi May 26, 2026 15:02
Comment thread src/turtle_kv/kv_store.cpp

//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - -
//
Status KVStore::sync(Optional<i64> upper_bound) noexcept
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.

Optional<i64> -> Optional<EditOffset>?

EditOffsetDelta{(i64)packed_key_value_slot_size(key, value)};
}

return OkStatus();
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.

Unreachable code?

BATT_REQUIRE_OK(this->art_index_.insert(key, inserter));
BATT_CHECK_NOT_NULLPTR(inserter.entry_out);

return inserter.entry_out->edit_offset() +
Copy link
Copy Markdown
Collaborator

@tonyastolfi tonyastolfi May 26, 2026

Choose a reason for hiding this comment

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

Data race: we should add a data member edit_offset_out(either edit_offset_upper_bound_out or Interval<EditOffset> edit_range_out) to MemTableValueEntryInserter

}
}();

this->sync_upper_bound_.close();
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.

Use batt::finally

// Insert this block's slots into the sync upper bound hash map.
//
for (usize i = 0; i < next_block->slot_count(); ++i) {
const i64 slot_start = next_block->slot_edit_offset(i).value();
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.

We can use ChangeLogBlock::edit_offset_range() here

@vidyasilai vidyasilai self-assigned this May 26, 2026
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.

2 participants