Skip to content

Commit e44dc66

Browse files
committed
ct/housekeeper: don't RPC if start offset hasn't changed
Noticed that set_start_offset RPCs were constantly being sent to the metastore, even for topics with a start offset had been 0 for a while. Functionally this isn't problematic, but it's noise to the cluster. This commit caches the last synced start offset so we can avoid the RPC. It's transient, so when the housekeeper is reset (e.g. on partition leadership change) we'll still see a potentially "wasted" no-op L1 set_start_offset call, but that's still better than what we have now.
1 parent 83e56bc commit e44dc66

3 files changed

Lines changed: 40 additions & 1 deletion

File tree

src/v/cloud_topics/housekeeper/housekeeper.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,9 @@ ss::future<> housekeeper::sync_start_offset() {
190190
co_return;
191191
}
192192
auto start_offset = _l0_metastore->get_start_offset(_tidp);
193+
if (_last_synced_start_offset == start_offset) {
194+
co_return;
195+
}
193196
vlog(cd_log.debug, "Setting {} start offset to {}", _tidp, start_offset);
194197
auto result = co_await _l1_metastore->set_start_offset(_tidp, start_offset);
195198
if (!result.has_value()) {
@@ -198,7 +201,9 @@ ss::future<> housekeeper::sync_start_offset() {
198201
"Failed to sync start offset to L1 for {}: {}",
199202
_tidp,
200203
result.error());
204+
co_return;
201205
}
206+
_last_synced_start_offset = start_offset;
202207
}
203208

204209
} // namespace cloud_topics

src/v/cloud_topics/housekeeper/housekeeper.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ class housekeeper {
144144
ss::abort_source _as;
145145

146146
std::optional<cloud_topics::cluster_epoch> _last_epoch;
147+
std::optional<kafka::offset> _last_synced_start_offset;
147148
};
148149

149150
} // namespace cloud_topics

src/v/cloud_topics/housekeeper/tests/housekeeper_test.cc

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,17 @@ T handle_error(
197197
std::to_underlying(result.error())));
198198
}
199199

200+
class counting_metastore : public cloud_topics::l1::simple_metastore {
201+
public:
202+
ss::future<std::expected<void, errc>> set_start_offset(
203+
const model::topic_id_partition& tidp, kafka::offset o) override {
204+
++set_start_offset_calls;
205+
return simple_metastore::set_start_offset(tidp, o);
206+
}
207+
208+
size_t set_start_offset_calls{0};
209+
};
210+
200211
} // namespace
201212

202213
using namespace std::chrono_literals;
@@ -224,6 +235,10 @@ class HousekeeperTest : public testing::Test {
224235
_l0_metastore.set_max_allowed_start_offset(offset);
225236
}
226237

238+
size_t l1_set_start_offset_calls() const {
239+
return _l1_metastore.set_start_offset_calls;
240+
}
241+
227242
kafka::offset l1_start_offset() {
228243
auto result = _l1_metastore.get_offsets(_tidp).get();
229244
if (!result.has_value()) {
@@ -289,7 +304,7 @@ class HousekeeperTest : public testing::Test {
289304
model::topic_id_partition _tidp{
290305
model::create_topic_id(), model::partition_id{0}};
291306
fake_l0_metastore _l0_metastore{_tidp, kafka::offset{0}};
292-
cloud_topics::l1::simple_metastore _l1_metastore;
307+
counting_metastore _l1_metastore;
293308
std::unique_ptr<retention_config_impl> _config_impl;
294309
};
295310

@@ -848,3 +863,21 @@ TEST_F(HousekeeperTest, BumpEpochMultipleIdleCycles) {
848863
EXPECT_EQ(advance_epoch_calls()[0], cloud_topics::cluster_epoch{10});
849864
EXPECT_EQ(sync_to_next_placeholder_calls(), 1);
850865
}
866+
867+
TEST_F(HousekeeperTest, SyncStartOffsetSkipsRedundantRPC) {
868+
auto housekeeper = make_housekeeper({});
869+
add_object({.records = 100, .size = 1_MiB});
870+
set_start_offset(kafka::offset{50});
871+
872+
housekeeper.do_housekeeping().get();
873+
EXPECT_EQ(l1_set_start_offset_calls(), 1);
874+
875+
// No change — should skip the call.
876+
housekeeper.do_housekeeping().get();
877+
EXPECT_EQ(l1_set_start_offset_calls(), 1);
878+
879+
// Changed offset — should sync again.
880+
set_start_offset(kafka::offset{75});
881+
housekeeper.do_housekeeping().get();
882+
EXPECT_EQ(l1_set_start_offset_calls(), 2);
883+
}

0 commit comments

Comments
 (0)