diff --git a/db/commitlog/commitlog.cc b/db/commitlog/commitlog.cc index 7670fb1e9b..8da35d743c 100644 --- a/db/commitlog/commitlog.cc +++ b/db/commitlog/commitlog.cc @@ -340,7 +340,9 @@ public: future<> oversized_allocation(entry_writer&, db::timeout_clock::time_point timeout); - replay_position min_position(); + replay_position min_position() const; + replay_position current_position() const; + size_t sector_overhead(segment_id_type, size_t) const; template struct byte_flow { @@ -1508,7 +1510,7 @@ public: } }; -db::replay_position db::commitlog::segment_manager::min_position() { +db::replay_position db::commitlog::segment_manager::min_position() const { if (_segments.empty()) { return {_ids, 0}; } else { @@ -1516,6 +1518,24 @@ db::replay_position db::commitlog::segment_manager::min_position() { } } +db::replay_position db::commitlog::segment_manager::current_position() const { + if (_segments.empty()) { + return {_ids, 0}; + } else { + auto s = _segments.front(); + return {s->_desc.id, s->position()}; + } +} + +size_t db::commitlog::segment_manager::sector_overhead(segment_id_type id, size_t size) const { + for (auto& s : _segments) { + if (s->_desc.id == id) { + return s->sector_overhead(size); + } + } + return 0; +} + template requires std::derived_from && std::same_as().result())> future db::commitlog::segment_manager::allocate_when_possible(T writer, db::timeout_clock::time_point timeout) { @@ -3964,6 +3984,14 @@ db::replay_position db::commitlog::min_position() const { return _segment_manager->min_position(); } +db::replay_position db::commitlog::current_position() const { + return _segment_manager->current_position(); +} + +size_t db::commitlog::sector_overhead(segment_id_type id, size_t size) const { + return _segment_manager->sector_overhead(id, size); +} + void db::commitlog::update_max_data_lifetime(std::optional commitlog_data_max_lifetime_in_seconds) { _segment_manager->cfg.commitlog_data_max_lifetime_in_seconds = commitlog_data_max_lifetime_in_seconds; } diff --git a/db/commitlog/commitlog.hh b/db/commitlog/commitlog.hh index 745c89bd4a..fbe3caa93b 100644 --- a/db/commitlog/commitlog.hh +++ b/db/commitlog/commitlog.hh @@ -382,6 +382,12 @@ public: // be replayed on the next reboot. replay_position min_position() const; + // For testing only. Returns the active segments current position + // (ignoring chunk overhead etc) + replay_position current_position() const; + // Gets sector overhead for size in given segment + size_t sector_overhead(segment_id_type, size_t) const; + // (Re-)set data mix lifetime. void update_max_data_lifetime(std::optional commitlog_data_max_lifetime_in_seconds); diff --git a/test/boost/commitlog_test.cc b/test/boost/commitlog_test.cc index 20c84092c0..51f055c1b9 100644 --- a/test/boost/commitlog_test.cc +++ b/test/boost/commitlog_test.cc @@ -2144,6 +2144,52 @@ SEASTAR_TEST_CASE(test_oversized_with_terminate_in_buffer_wait) { }, exts.get()); } + +SEASTAR_TEST_CASE(test_oversized_at_segment_boundary) { + co_await test_oversized(1, 1, [&](commitlog& log) -> future<> { + // Add an entry that is exactly segment_size - chunk+entry overhead. + // provoking segment overflow. + auto uuid = make_table_id(); + auto max_size = log.max_record_size(); + auto buf = fragmented_temporary_buffer::allocate_to_fit(max_size); + auto h = co_await log.add_mutation(uuid, buf.size_bytes(), db::commitlog::force_sync::no, [&](db::commitlog::output& dst) { + for (auto& tmp : buf) { + dst.write(tmp.get(), tmp.size()); + } + }); + + co_await log.sync_all_segments(); + auto pos = log.current_position(); + + BOOST_CHECK_EQUAL(h.rp().id, pos.id); + BOOST_CHECK_LT(h.rp().pos, pos.pos); + + // TODO: maybe export this so we don't know it here as well... + auto overhead = 2 * sizeof(uint32_t); + auto base_size = 1024*1024 - pos.pos - overhead; + auto sector_overhead = log.sector_overhead(pos.id, base_size); + + BOOST_CHECK_GT(sector_overhead, 0); + + // With bug, we get to write this in same segment as above, even though we overshoot position now + buf = fragmented_temporary_buffer::allocate_to_fit(base_size - sector_overhead); + auto h2 = co_await log.add_mutation(uuid, buf.size_bytes(), db::commitlog::force_sync::no, [&](db::commitlog::output& dst) { + for (auto& tmp : buf) { + dst.write(tmp.get(), tmp.size()); + } + }); + + // Should have gone to new segment. + BOOST_CHECK_NE(h2.rp().id, pos.id); + + h.release(); + h2.release(); + + // Now, if bugged, oversized test will overflow size calc and crash. + // With test, it will not. + }); +} + // tests #20862 - buffer usage counter not being updated correctly SEASTAR_TEST_CASE(test_commitlog_buffer_size_counter) { commitlog::config cfg;