-
Notifications
You must be signed in to change notification settings - Fork 602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Iceberg backlog controller #24990
Iceberg backlog controller #24990
Changes from all commits
af9ee3b
74fcf7c
6681b54
bf78e62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
/* | ||
* Copyright 2025 Redpanda Data, Inc. | ||
* | ||
* Licensed as a Redpanda Enterprise file under the Redpanda Community | ||
* License (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* https://github.com/redpanda-data/redpanda/blob/master/licenses/rcl.md | ||
*/ | ||
#include "datalake/backlog_controller.h" | ||
|
||
#include "base/vlog.h" | ||
#include "config/configuration.h" | ||
#include "datalake/logger.h" | ||
#include "metrics/prometheus_sanitize.h" | ||
|
||
#include <seastar/core/coroutine.hh> | ||
#include <seastar/core/metrics.hh> | ||
#include <seastar/core/reactor.hh> | ||
using namespace std::chrono_literals; // NOLINT | ||
|
||
namespace datalake { | ||
|
||
backlog_controller::backlog_controller( | ||
sampling_fn sampling_fn, ss::scheduling_group sg) | ||
: _sampling_f(std::move(sampling_fn)) | ||
, _scheduling_group(sg) | ||
, _proportional_coeff( | ||
config::shard_local_cfg().iceberg_backlog_controller_p_coeff.bind()) | ||
, _setpoint(config::shard_local_cfg().iceberg_target_backlog_size.bind()) | ||
, _sampling_interval(5s) {} | ||
|
||
ss::future<> backlog_controller::start() { | ||
setup_metrics(); | ||
_sampling_timer.set_callback([this] { | ||
update(); | ||
if (!_as.abort_requested()) { | ||
_sampling_timer.arm(_sampling_interval); | ||
} | ||
}); | ||
|
||
_sampling_timer.arm(_sampling_interval); | ||
co_return; | ||
} | ||
|
||
ss::future<> backlog_controller::stop() { | ||
_sampling_timer.cancel(); | ||
_as.request_abort(); | ||
co_return; | ||
} | ||
|
||
void backlog_controller::update() { | ||
using namespace std::chrono_literals; | ||
|
||
_current_sample = _sampling_f(); | ||
|
||
auto current_err = _setpoint() - _current_sample; | ||
auto update = _proportional_coeff() * current_err; | ||
|
||
update = std::clamp(static_cast<int>(update), _min_shares, _max_shares); | ||
|
||
vlog( | ||
datalake_log.trace, | ||
"state update: {{setpoint: {}, current_backlog: {:2f}, current_error: " | ||
"{:2f}, shares_update: {:2f}}}", | ||
_setpoint(), | ||
_current_sample, | ||
current_err, | ||
update); | ||
|
||
_scheduling_group.set_shares(static_cast<float>(update)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any use to just adjust the weights of translation and produce, something like translation_shares + delta In the current form I think it shuffles the entire share distribution which effectively means (for example) fetch gets fewer shares if there is a translation lag, is that the intention? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is good question. What we change is the ratio between the |
||
} | ||
|
||
void backlog_controller::setup_metrics() { | ||
if (config::shard_local_cfg().disable_metrics()) { | ||
return; | ||
} | ||
namespace sm = ss::metrics; | ||
_metrics.add_group( | ||
prometheus_sanitize::metrics_name("iceberg:backlog:controller"), | ||
{ | ||
sm::make_gauge( | ||
"backlog_size", | ||
[this] { return _current_sample; }, | ||
sm::description("Iceberg controller current backlog - averaged size " | ||
"of the backlog per partition")), | ||
|
||
}); | ||
} | ||
|
||
} // namespace datalake |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* Copyright 2025 Redpanda Data, Inc. | ||
* | ||
* Licensed as a Redpanda Enterprise file under the Redpanda Community | ||
* License (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* https://github.com/redpanda-data/redpanda/blob/master/licenses/rcl.md | ||
*/ | ||
|
||
#pragma once | ||
#include "base/seastarx.h" | ||
#include "config/property.h" | ||
#include "metrics/metrics.h" | ||
|
||
#include <seastar/core/gate.hh> | ||
#include <seastar/core/io_priority_class.hh> | ||
#include <seastar/core/metrics_registration.hh> | ||
#include <seastar/core/timer.hh> | ||
#include <seastar/util/log.hh> | ||
|
||
namespace datalake { | ||
/** | ||
* Class responsible for adjusting shares to maintain a target translation | ||
* backlog size. This simple proportional controller uses a sampling function to | ||
* collect the size of the backlog and adjust the shares of the datalake | ||
* scheduling group to keep the average backlog size close to the setpoint. | ||
*/ | ||
class backlog_controller { | ||
public: | ||
using sampling_fn = ss::noncopyable_function<long double()>; | ||
backlog_controller(sampling_fn, ss::scheduling_group sg); | ||
|
||
ss::future<> start(); | ||
ss::future<> stop(); | ||
|
||
private: | ||
void update(); | ||
void setup_metrics(); | ||
|
||
sampling_fn _sampling_f; | ||
ss::scheduling_group _scheduling_group; | ||
/** | ||
* Proportionality coefficient for the controller, this value controls how | ||
* eager the controller is to adjust the shares of the datalake scheduling | ||
* group. | ||
*/ | ||
config::binding<double> _proportional_coeff; | ||
/** | ||
* The target backlog size for the controller to keep. The controller will | ||
* adjust the scheduling group shares keep the average backlog size close to | ||
* this value. | ||
*/ | ||
config::binding<uint32_t> _setpoint; | ||
std::chrono::steady_clock::duration _sampling_interval; | ||
ss::timer<> _sampling_timer; | ||
// Current value of the backlog (currently it is average size of data to | ||
// translate for datalake enabled partitions) | ||
long double _current_sample{0}; | ||
/** | ||
* Limits for controlled scheduling group shares. | ||
*/ | ||
int _min_shares{1}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wondering if min should be at least default 100. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think not necessary, if the translator can keep up with the work with the lowest possible priority we can use it, otherwise the priority will be increased. From my experiments when producer throughput fluctuates even 1 share is enough to promptly translate the remaining backlog when ingest rate is lower. |
||
int _max_shares{1000}; | ||
ss::abort_source _as; | ||
metrics::internal_metric_groups _metrics; | ||
}; | ||
} // namespace datalake |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -340,6 +340,32 @@ partition_translator::do_translation_for_range( | |
co_return std::move(result.value()); | ||
} | ||
|
||
std::optional<size_t> partition_translator::translation_backlog() const { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious if there's actually correlation between translation time and bytes on disk, particularly in the face of compressed batches, or complex schemas. I guess the thinking is larger messages likely imply more complex payload, but just thinking out loud I wonder if something as simple as number of records pending would be usable here. Not really advocating for any changes, but just thought I'd pose the question, and also point out that this won't work for read replicas or restored/FPM-ed partitions without implementing a similar sizing method for cloud. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I strongly agree with your point i.e. the actual amount of work is definitely dependent from schema complexity, compression, message size, etc. I think this doesn't really change the relation that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is what I'm curious about specifically: if two records have the same schema but one has 1 byte per field while the other has 100 bytes per field, does the latter end up taking meaningfully more CPU for parquet writing? I guess size affects things like comparisons, and flush frequency maybe. Just curious if we know if translation is dominated by those factors, vs schema parsing. |
||
if (!_last_translated_log_offset.has_value()) { | ||
return std::nullopt; | ||
} | ||
|
||
auto size_after_translated = _partition->log()->size_bytes_after_offset( | ||
_last_translated_log_offset.value()); | ||
|
||
auto size_after_max_translatable | ||
= _partition->log()->size_bytes_after_offset( | ||
_partition->last_stable_offset()); | ||
|
||
if (size_after_translated < size_after_max_translatable) { | ||
vlog( | ||
_logger.error, | ||
"Expected size after translated offset({}) {} to be greater than or " | ||
"equal to the size of log after max translatable offset({}): {}", | ||
_last_translated_log_offset, | ||
size_after_translated, | ||
_partition->last_stable_offset()); | ||
return std::nullopt; | ||
} | ||
|
||
return size_after_translated - size_after_max_translatable; | ||
} | ||
|
||
ss::future<partition_translator::translation_success> | ||
partition_translator::do_translate_once(retry_chain_node& parent_rcn) { | ||
if ( | ||
|
@@ -399,6 +425,9 @@ partition_translator::do_translate_once(retry_chain_node& parent_rcn) { | |
read_begin_offset, | ||
std::move(translation_result.value()))) { | ||
max_translated_offset = last_translated_offset; | ||
_last_translated_log_offset | ||
= _partition->get_offset_translator_state()->to_log_offset( | ||
kafka::offset_cast(max_translated_offset)); | ||
result = translation_success::yes; | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was initially considering whether the default size should be slightly larger than 5 MiB. However, a smaller default has the advantage of smaller proportional changes to scheduling weights. This allows the system to correct itself more quickly than with a larger default (e.g., 1 GiB), where weight changes would be drastic and throttling more significant (bigger sawtooth). Therefore, we likely prefer smaller values for faster system adjustment, am I thinking about it correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason i used the small value here is a bit different. I think the absolute change of the backlog size will always be the same doesn't matter if the setpoint is 5_MiB or 1_GiB. I set the value small to promote reading data from
batch_cache
in my experiments the larger the value the higher the rate of cache misses.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm ya, batch cache hits reasoning make sense.