From f9397aef821b7bee8a8d715b575880839392e796 Mon Sep 17 00:00:00 2001 From: Anthony Astolfi Date: Mon, 29 Jul 2024 15:10:37 -0400 Subject: [PATCH 1/3] Add more metrics to IoRingLogDevice2 and verify them in the sim test. From 8c9d715c1ba89c567db18f88fd0ff4421e00389a Mon Sep 17 00:00:00 2001 From: Anthony Astolfi Date: Tue, 30 Jul 2024 16:39:52 -0400 Subject: [PATCH 2/3] Remove atomic slot range tokens. --- src/llfs/slot_writer.cpp | 56 -------------------------------- src/llfs/slot_writer.hpp | 37 --------------------- src/llfs/slot_writer.test.cpp | 23 ------------- src/llfs/volume_multi_append.hpp | 17 +--------- 4 files changed, 1 insertion(+), 132 deletions(-) diff --git a/src/llfs/slot_writer.cpp b/src/llfs/slot_writer.cpp index d69366c..035bfb9 100644 --- a/src/llfs/slot_writer.cpp +++ b/src/llfs/slot_writer.cpp @@ -57,23 +57,6 @@ void SlotWriter::halt() this->pool_.close(); } -//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - -// -/*static*/ Slice SlotWriter::WriterLock::begin_atomic_range_token() noexcept -{ - const static std::array token_ = {0b10000000, 0b10000000, - 0b00000000}; - return as_const_slice(token_); -} - -//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - -// -/*static*/ Slice SlotWriter::WriterLock::end_atomic_range_token() noexcept -{ - const static std::array token_ = {0b10000000, 0b00000000}; - return as_const_slice(token_); -} - //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // /*explicit*/ SlotWriter::WriterLock::WriterLock(SlotWriter& slot_writer) noexcept @@ -83,45 +66,6 @@ void SlotWriter::halt() BATT_CHECK_NOT_NULLPTR(*this->writer_lock_); } -//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - -// -Status SlotWriter::WriterLock::append_token_impl(const Slice& token, - const batt::Grant& caller_grant) noexcept -{ - BATT_CHECK_EQ(this->prepare_size_, 0u); - - const usize prepare_size = this->deferred_commit_size_ + token.size(); - - if (caller_grant.size() < prepare_size) { - return ::llfs::make_status(StatusCode::kSlotGrantTooSmall); - } - - BATT_ASSIGN_OK_RESULT(MutableBuffer buffer, - (*this->writer_lock_)->prepare(prepare_size, /*head_room=*/0)); - - buffer += this->deferred_commit_size_; - - std::memcpy(buffer.data(), token.begin(), token.size()); - - this->deferred_commit_size_ += token.size(); - - return batt::OkStatus(); -} - -//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - -// -Status SlotWriter::WriterLock::begin_atomic_range(const batt::Grant& caller_grant) noexcept -{ - return this->append_token_impl(Self::begin_atomic_range_token(), caller_grant); -} - -//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - -// -Status SlotWriter::WriterLock::end_atomic_range(const batt::Grant& caller_grant) noexcept -{ - return this->append_token_impl(Self::end_atomic_range_token(), caller_grant); -} - //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // StatusOr SlotWriter::WriterLock::prepare(usize slot_payload_size, diff --git a/src/llfs/slot_writer.hpp b/src/llfs/slot_writer.hpp index 8b50eba..a55ae04 100644 --- a/src/llfs/slot_writer.hpp +++ b/src/llfs/slot_writer.hpp @@ -136,12 +136,6 @@ class SlotWriter::WriterLock public: using Self = WriterLock; - static constexpr usize kBeginAtomicRangeTokenSize = 3; - static constexpr usize kEndAtomicRangeTokenSize = 2; - - static Slice begin_atomic_range_token() noexcept; - static Slice end_atomic_range_token() noexcept; - //+++++++++++-+-+--+----- --- -- - - - - /** \brief Acquires an exclusive lock on writing to the log managed by `slot_writer`, preparing @@ -159,19 +153,6 @@ class SlotWriter::WriterLock //+++++++++++-+-+--+----- --- -- - - - - - /** \brief Appends special token to the log to indicate the start of a sequence of slots that - * must commit/recover atomically. - * - * The token commit is deferred, requiring later calls to end_atomic_range and commit. - */ - Status begin_atomic_range(const batt::Grant& caller_grant) noexcept; - - /** \brief Appends special sequence to the log indicating the end of the current atomic range. - * - * The token commit is deferred, requiring a later call to commit. - */ - Status end_atomic_range(const batt::Grant& caller_grant) noexcept; - /** \brief Verifies that the passed grant can accomodate a new slot with the passed payload size * (in addition to any currently deferred slots), then allocates space in the log and writes a * header for the new slot, returning a mutable buffer for _just_ the payload portion. @@ -206,14 +187,6 @@ class SlotWriter::WriterLock //+++++++++++-+-+--+----- --- -- - - - - private: - /** \brief Appends the specified byte range to the prepared data. `token` is just a raw byte - * sequence, not a full slot. This function is used to implement the `begin_atomic_range` and - * `end_atomic_range` tokens. - */ - Status append_token_impl(const Slice& token, const batt::Grant& caller_grant) noexcept; - - //+++++++++++-+-+--+----- --- -- - - - - - /** \brief The SlotWriter object passed in at construction time. */ SlotWriter& slot_writer_; @@ -289,16 +262,6 @@ class TypedSlotWriter> : public SlotWriter //----- --- -- - - - - - Status begin_atomic_range(const batt::Grant& caller_grant) noexcept - { - return this->writer_lock_.begin_atomic_range(caller_grant); - } - - Status end_atomic_range(const batt::Grant& caller_grant) noexcept - { - return this->writer_lock_.end_atomic_range(caller_grant); - } - /** \brief Packs a single slot into the log device buffer. Does not commit the slot; all slots * are committed atomically when this->finalize() is called. */ diff --git a/src/llfs/slot_writer.test.cpp b/src/llfs/slot_writer.test.cpp index 2cdb396..2272683 100644 --- a/src/llfs/slot_writer.test.cpp +++ b/src/llfs/slot_writer.test.cpp @@ -19,27 +19,4 @@ namespace { using namespace llfs::int_types; -TEST(SlotWriterTest, BeginEndAtomicRangeTokens) -{ - llfs::Slice begin_token = llfs::SlotWriter::WriterLock::begin_atomic_range_token(); - llfs::Slice end_token = llfs::SlotWriter::WriterLock::end_atomic_range_token(); - - EXPECT_EQ(begin_token.size(), llfs::SlotWriter::WriterLock::kBeginAtomicRangeTokenSize); - EXPECT_EQ(end_token.size(), llfs::SlotWriter::WriterLock::kEndAtomicRangeTokenSize); - EXPECT_NE(begin_token.size(), end_token.size()); - EXPECT_GT(begin_token.size(), 1u); - EXPECT_GT(end_token.size(), 1u); - - const auto verify_parses_as_zero = [](const llfs::Slice& token) { - auto [parsed_n, end_of_parse] = llfs::unpack_varint_from(token.begin(), token.end()); - - ASSERT_TRUE(parsed_n); - EXPECT_EQ(*parsed_n, 0u); - EXPECT_EQ(end_of_parse, token.end()); - }; - - verify_parses_as_zero(begin_token); - verify_parses_as_zero(end_token); -} - } // namespace diff --git a/src/llfs/volume_multi_append.hpp b/src/llfs/volume_multi_append.hpp index 06581cf..8682660 100644 --- a/src/llfs/volume_multi_append.hpp +++ b/src/llfs/volume_multi_append.hpp @@ -35,8 +35,7 @@ class VolumeMultiAppend */ static constexpr u64 calculate_grant_size(u64 slots_total_size) noexcept { - return slots_total_size + SlotWriter::WriterLock::kBeginAtomicRangeTokenSize + - SlotWriter::WriterLock::kEndAtomicRangeTokenSize; + return slots_total_size; } //+++++++++++-+-+--+----- --- -- - - - - @@ -85,11 +84,6 @@ class VolumeMultiAppend { BATT_CHECK(!this->closed_); - if (this->first_) { - this->first_ = false; - BATT_REQUIRE_OK(this->op_.begin_atomic_range(grant)); - } - llfs::PackObjectAsRawData packed_obj_as_raw{payload}; return this->op_.append(grant, packed_obj_as_raw); @@ -115,10 +109,6 @@ class VolumeMultiAppend { BATT_CHECK(!this->closed_); - if (!this->first_) { - BATT_REQUIRE_OK(this->op_.end_atomic_range(grant)); - } - StatusOr result = this->op_.finalize(grant); BATT_REQUIRE_OK(result); @@ -142,11 +132,6 @@ class VolumeMultiAppend // TypedSlotWriter::MultiAppend op_; - // true iff no call append has yet been made; used to determine when we should write a begin - // atomic range token to the log. - // - bool first_ = true; - // true iff commit or cancel has been called. // bool closed_ = false; From e8922f6e6d3350576927dc2f0fb57fb286c5cd4d Mon Sep 17 00:00:00 2001 From: Anthony Astolfi Date: Thu, 1 Aug 2024 14:46:48 -0400 Subject: [PATCH 3/3] Fix for issue/159. --- conanfile.py | 2 +- src/llfs/basic_ring_buffer_log_device.hpp | 6 +++++- src/llfs/ioring_log_device2.hpp | 1 + src/llfs/ioring_log_device2.test.cpp | 10 ++++++---- src/llfs/volume.cpp | 9 ++++++++- src/llfs/volume.hpp | 4 ++++ src/llfs/volume_trimmer.cpp | 12 ++++++------ 7 files changed, 31 insertions(+), 13 deletions(-) diff --git a/conanfile.py b/conanfile.py index 48a6060..c80ccea 100644 --- a/conanfile.py +++ b/conanfile.py @@ -69,7 +69,7 @@ def configure(self): def requirements(self): - self.requires("batteries/0.53.0", **VISIBLE) + self.requires("batteries/0.55.1", **VISIBLE) self.requires("boost/1.83.0", **VISIBLE) self.requires("cli11/2.3.2", **VISIBLE) self.requires("glog/0.6.0", **VISIBLE) diff --git a/src/llfs/basic_ring_buffer_log_device.hpp b/src/llfs/basic_ring_buffer_log_device.hpp index 7afaea9..823ebac 100644 --- a/src/llfs/basic_ring_buffer_log_device.hpp +++ b/src/llfs/basic_ring_buffer_log_device.hpp @@ -245,7 +245,11 @@ class BasicRingBufferLogDevice::WriterImpl : public LogDevice::Writer const usize space_required = byte_count + head_room; if (this->space() < space_required) { - return ::llfs::make_status(StatusCode::kPrepareFailedTrimRequired); + BATT_REQUIRE_OK(::llfs::make_status(StatusCode::kPrepareFailedTrimRequired)) + << BATT_INSPECT(space_required) << BATT_INSPECT(this->space()) + << BATT_INSPECT(this->device_->driver_.get_trim_pos()) + << BATT_INSPECT(this->device_->driver_.get_commit_pos()) + << boost::stacktrace::stacktrace{}; } const slot_offset_type commit_pos = this->device_->driver_.get_commit_pos(); diff --git a/src/llfs/ioring_log_device2.hpp b/src/llfs/ioring_log_device2.hpp index 6f54cbc..7dbfb60 100644 --- a/src/llfs/ioring_log_device2.hpp +++ b/src/llfs/ioring_log_device2.hpp @@ -83,6 +83,7 @@ class IoRingLogDriver2 Status set_trim_pos(slot_offset_type trim_pos) { this->observed_watch_[kTargetTrimPos].set_value(trim_pos); + BATT_REQUIRE_OK(this->await_trim_pos(trim_pos)); return OkStatus(); } diff --git a/src/llfs/ioring_log_device2.test.cpp b/src/llfs/ioring_log_device2.test.cpp index b7a4bca..56a0d63 100644 --- a/src/llfs/ioring_log_device2.test.cpp +++ b/src/llfs/ioring_log_device2.test.cpp @@ -36,7 +36,7 @@ using llfs::StatusOr; // the confirmed trim and flush never go backwards, and that all confirmed flushed data can be // read. // -class IoringLogDevice2SimTest : public ::testing::Test +class IoRingLogDevice2SimTest : public ::testing::Test { public: static constexpr usize kNumSeeds = 250 * 1000; @@ -326,6 +326,8 @@ class IoringLogDevice2SimTest : public ::testing::Test if (!trim_status.ok()) { break; } + BATT_CHECK_EQ(this->log_device->slot_range(llfs::LogReadMode::kDurable).lower_bound, + trimmed_slot.upper_bound); Status await_status = this->log_writer->await(llfs::BytesAvailable{ .size = trimmed_slot.size() + observed_space, @@ -503,11 +505,11 @@ class IoringLogDevice2SimTest : public ::testing::Test }; // struct Scenario -}; // class IoringLogDevice2SimTest +}; // class IoRingLogDevice2SimTest //=#=#==#==#===============+=+=+=+=++=++++++++++++++-++-+--+-+----+--------------- -TEST_F(IoringLogDevice2SimTest, Simulation) +TEST_F(IoRingLogDevice2SimTest, Simulation) { const usize kNumThreads = std::thread::hardware_concurrency(); const usize kUpdateInterval = kNumSeeds / 20; @@ -558,7 +560,7 @@ TEST_F(IoringLogDevice2SimTest, Simulation) //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // -TEST(IoringLogDevice2Test, Benchmark) +TEST(IoRingLogDevice2Test, Benchmark) { //+++++++++++-+-+--+----- --- -- - - - - // Set configuration and options. diff --git a/src/llfs/volume.cpp b/src/llfs/volume.cpp index 2419c4c..18c2a7b 100644 --- a/src/llfs/volume.cpp +++ b/src/llfs/volume.cpp @@ -39,6 +39,13 @@ const VolumeOptions& Volume::options() const return this->options_; } +//==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - +// +const std::string& Volume::name() const noexcept +{ + return this->options().name; +} + //==#==========+==+=+=++=+++++++++++-+-+--+----- --- -- - - - - // const boost::uuids::uuid& Volume::get_volume_uuid() const @@ -374,7 +381,7 @@ void Volume::start() Status result = this->trimmer_->run(); LLFS_VLOG(1) << "Volume::trimmer_task_ exited with status=" << result; }, - "Volume::trimmer_task_"); + /*task_name=*/batt::to_string(this->name(), "_Volume.trimmer_task")); } } diff --git a/src/llfs/volume.hpp b/src/llfs/volume.hpp index a5e4d24..67fb795 100644 --- a/src/llfs/volume.hpp +++ b/src/llfs/volume.hpp @@ -88,6 +88,10 @@ class Volume // const VolumeOptions& options() const; + /** \brief The name for this Volume, as specified by the VolumeOptions. + */ + const std::string& name() const noexcept; + // Returns the UUID for this volume. // const boost::uuids::uuid& get_volume_uuid() const; diff --git a/src/llfs/volume_trimmer.cpp b/src/llfs/volume_trimmer.cpp index 4205aa3..90ad7f4 100644 --- a/src/llfs/volume_trimmer.cpp +++ b/src/llfs/volume_trimmer.cpp @@ -217,12 +217,12 @@ Status VolumeTrimmer::run() //+++++++++++-+-+--+----- --- -- - - - - // Wait for the trim point to advance. // - BATT_DEBUG_INFO("waiting for trim target to advance; " - << BATT_INSPECT(this->trim_control_.get_lower_bound()) - << BATT_INSPECT(least_upper_bound) << BATT_INSPECT(bytes_trimmed) - << BATT_INSPECT(trim_lower_bound) << std::endl - << BATT_INSPECT(this->trim_control_.debug_info()) << BATT_INSPECT(loop_counter) - << BATT_INSPECT(this->trim_control_.is_closed())); + BATT_DEBUG_INFO( + "waiting for trim target to advance; " + << BATT_INSPECT(this->trim_control_.get_lower_bound()) << BATT_INSPECT(least_upper_bound) + << BATT_INSPECT(bytes_trimmed) << BATT_INSPECT(trim_lower_bound) << std::endl + << BATT_INSPECT(this->trim_control_.debug_info()) << BATT_INSPECT(loop_counter) + << BATT_INSPECT(this->trim_control_.is_closed()) << BATT_INSPECT(this->trim_delay_)); StatusOr trim_upper_bound = this->await_trim_target(least_upper_bound);