fsync Implementation#23
Conversation
|
|
||
| //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - | ||
| // | ||
| Status KVStore::sync(Optional<i64> upper_bound) noexcept |
There was a problem hiding this comment.
Optional<i64> -> Optional<EditOffset>?
| EditOffsetDelta{(i64)packed_key_value_slot_size(key, value)}; | ||
| } | ||
|
|
||
| return OkStatus(); |
| BATT_REQUIRE_OK(this->art_index_.insert(key, inserter)); | ||
| BATT_CHECK_NOT_NULLPTR(inserter.entry_out); | ||
|
|
||
| return inserter.entry_out->edit_offset() + |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Done, added edit_range_out.
| } | ||
| }(); | ||
|
|
||
| this->sync_upper_bound_.close(); |
There was a problem hiding this comment.
As per our conversation earlier, I added this to ChangeLogWriter::halt.
| // 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(); |
There was a problem hiding this comment.
We can use ChangeLogBlock::edit_offset_range() here
| * | ||
| * \return The new edit_offset_start value after walking. | ||
| */ | ||
| template <typename SlotFn> |
There was a problem hiding this comment.
Let's use concepts/type requirements here to specify the signature of the Fn.
There was a problem hiding this comment.
Done (see src/turtle_kv/change_log/change_log_blocks_visitor.hpp).
| * \return The new edit_offset_start value after walking. | ||
| */ | ||
| template <typename SlotFn> | ||
| i64 walk_change_log_blocks(i64 edit_offset_start, |
| pending_blocks.erase(it); | ||
|
|
||
| do { | ||
| slot_fn(block_iter.block.get(), block_iter.next_slot_i, EditOffset{edit_offset_start}); |
There was a problem hiding this comment.
The slot fn should probably have a 'this-is-the-first-visit' arg. Also good to have it optionally return Status; there are break-loop and continue-loop enum values we can use here: https://gitlab.com/batteriescpp/batteries/-/blob/main/src/batteries/seq/loop_control.hpp?ref_type=heads#L123 (BATT_INVOKE_LOOP_FN)
| do { | ||
| slot_fn(block_iter.block.get(), block_iter.next_slot_i, EditOffset{edit_offset_start}); | ||
|
|
||
| edit_offset_start += (i64)(block_iter.block->slot_size(block_iter.next_slot_i) - |
There was a problem hiding this comment.
It would be great to add a function like ChangeLogBlock::next_edit_offset_of_slot(usize slot_index) that hides this implementation detail.
| // | ||
| if ((force_sleep || prepared.empty()) && this->halt_requested_.load() == false) { | ||
| if ((force_sleep || prepared.empty()) && this->halt_requested_.load() == false && | ||
| this->sync_upper_bound_.get_value() >= this->next_edit_offset_.load()) { |
There was a problem hiding this comment.
This expression would make a great public member function on ChangeLogWriter: i64 unflushed_byte_count()/bool has_unflushed_bytes() or similar.
| // | ||
| if ((force_sleep || prepared.empty()) && this->halt_requested_.load() == false) { | ||
| if ((force_sleep || prepared.empty()) && this->halt_requested_.load() == false && | ||
| this->sync_upper_bound_.get_value() >= this->next_edit_offset_.load()) { |
There was a problem hiding this comment.
We probably don't want to prevent sleeping any time there is unflushed data; only when in "urgent" mode.
2 participants
This PR adds an
fsynclike blocking call functionality for clients to ensure updates are durable in the write ahead log.