CORE-8928: Introduce redpanda.iceberg.target.lag.ms topic propery #25056

oleiman · 2025-02-06T23:04:00Z

No-op for now, but we'll use this once translation is ported over to the new scheduler.

Backports Required

Release Notes

Improvements

Introduce iceberg_target_lag_ms topic property

oleiman · 2025-02-06T23:04:12Z

/dt

oleiman · 2025-02-07T00:38:05Z

/dt

oleiman · 2025-02-07T03:32:42Z

/dt

oleiman · 2025-02-07T06:12:27Z

/dt

vbotbuildovich · 2025-02-07T09:42:22Z

Retry command for Build#61716

please wait until all jobs are finished before running the slash command



/ci-repeat 1
tests/rptest/tests/cluster_config_test.py::ClusterConfigTest.test_valid_settings

vbotbuildovich · 2025-02-07T10:36:36Z

CI test results

test results on build#61716

test_id	test_kind	job_url	test_status	passed
rptest.tests.cluster_config_test.ClusterConfigTest.test_valid_settings	ducktape	https://buildkite.com/redpanda/redpanda/builds/61716#0194df68-ff14-44cc-baa5-cf24c04a8e79	FAIL	0/20
rptest.tests.cluster_config_test.ClusterConfigTest.test_valid_settings	ducktape	https://buildkite.com/redpanda/redpanda/builds/61716#0194df6c-990f-4d71-9059-8fd636b80940	FAIL	0/20
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery	ducktape	https://buildkite.com/redpanda/redpanda/builds/61716#0194df68-ff15-45cb-9951-725a5547f25a	FLAKY	1/2
rptest.tests.compaction_recovery_test.CompactionRecoveryUpgradeTest.test_index_recovery_after_upgrade	ducktape	https://buildkite.com/redpanda/redpanda/builds/61716#0194df68-ff16-421e-9761-d628ad70ddff	FLAKY	1/2
rptest.tests.datalake.compaction_test.CompactionGapsTest.test_translation_no_gaps.cloud_storage_type=CloudStorageType.S3.catalog_type=CatalogType.REST_HADOOP	ducktape	https://buildkite.com/redpanda/redpanda/builds/61716#0194df6c-9910-44ba-a87a-5957140c30b1	FLAKY	1/2
rptest.tests.partition_movement_test.SIPartitionMovementTest.test_shadow_indexing.num_to_upgrade=0.cloud_storage_type=CloudStorageType.S3	ducktape	https://buildkite.com/redpanda/redpanda/builds/61716#0194df6c-9910-44ba-a87a-5957140c30b1	FLAKY	1/2
rptest.transactions.producers_api_test.ProducersAdminAPITest.test_producers_state_api_during_load	ducktape	https://buildkite.com/redpanda/redpanda/builds/61716#0194df68-ff15-45cb-9951-725a5547f25a	FLAKY	1/2

test results on build#61737

test_id	test_kind	job_url	test_status	passed
rptest.tests.cluster_config_test.ClusterConfigTest.test_valid_settings	ducktape	https://buildkite.com/redpanda/redpanda/builds/61737#0194e1a2-3591-4647-8f52-f5a2bfb9c69a	FAIL	0/20
rptest.tests.cluster_config_test.ClusterConfigTest.test_valid_settings	ducktape	https://buildkite.com/redpanda/redpanda/builds/61737#0194e1a7-b163-45d0-8605-095bc1cb6458	FAIL	0/20
rptest.tests.compaction_recovery_test.CompactionRecoveryUpgradeTest.test_index_recovery_after_upgrade	ducktape	https://buildkite.com/redpanda/redpanda/builds/61737#0194e1a2-3593-4acc-9b9a-b340d60a3243	FLAKY	1/2
rptest.tests.datalake.compaction_test.CompactionGapsTest.test_translation_no_gaps.cloud_storage_type=CloudStorageType.S3.catalog_type=CatalogType.REST_JDBC	ducktape	https://buildkite.com/redpanda/redpanda/builds/61737#0194e1a7-b162-4aee-a6bb-27aa58800718	FLAKY	1/3
rptest.transactions.consumer_offsets_test.VerifyConsumerOffsets.test_consumer_group_offsets	ducktape	https://buildkite.com/redpanda/redpanda/builds/61737#0194e1a2-3590-44d0-8ca0-3630c73dad33	FLAKY	1/3

test results on build#61744

test_id	test_kind	job_url	test_status	passed
rptest.tests.compaction_recovery_test.CompactionRecoveryUpgradeTest.test_index_recovery_after_upgrade	ducktape	https://buildkite.com/redpanda/redpanda/builds/61744#0194e298-ec17-47b0-8b47-a842471064c9	FLAKY	1/2
rptest.tests.datalake.compaction_test.CompactionGapsTest.test_translation_no_gaps.cloud_storage_type=CloudStorageType.S3.catalog_type=CatalogType.REST_HADOOP	ducktape	https://buildkite.com/redpanda/redpanda/builds/61744#0194e2b4-2d3b-478a-bb8d-42f11b95d41f	FLAKY	1/4
rptest.tests.datalake.datalake_dlq_test.DatalakeDLQTest.test_invalid_record_action.cloud_storage_type=CloudStorageType.S3.query_engine=QueryEngineType.SPARK.use_topic_property=True.action=drop	ducktape	https://buildkite.com/redpanda/redpanda/builds/61744#0194e298-ec17-49f9-b328-0eca8eaeca69	FLAKY	1/2
rptest.tests.topic_recovery_test.TopicRecoveryTest.test_prevent_recovery.cloud_storage_type=CloudStorageType.S3	ducktape	https://buildkite.com/redpanda/redpanda/builds/61744#0194e298-ec15-468e-83e1-abd20177be90	FLAKY	1/2

test results on build#61777

test_id	test_kind	job_url	test_status	passed
rptest.tests.compaction_recovery_test.CompactionRecoveryUpgradeTest.test_index_recovery_after_upgrade	ducktape	https://buildkite.com/redpanda/redpanda/builds/61777#0194f145-135a-4aab-a43a-5130e560491d	FLAKY	1/2
rptest.tests.datalake.compaction_test.CompactionGapsTest.test_translation_no_gaps.cloud_storage_type=CloudStorageType.S3.catalog_type=CatalogType.REST_JDBC	ducktape	https://buildkite.com/redpanda/redpanda/builds/61777#0194f14a-2ebc-453b-85f7-a7dd87f8a74a	FLAKY	1/2
rptest.tests.e2e_shadow_indexing_test.ShadowIndexingWhileBusyTest.test_create_or_delete_topics_while_busy.short_retention=True.cloud_storage_type=CloudStorageType.ABS	ducktape	https://buildkite.com/redpanda/redpanda/builds/61777#0194f14a-2ebf-4ad7-9645-5259e9cc5d22	FLAKY	1/2

oleiman · 2025-02-07T17:02:49Z

@bharathv - I have a few todos sprinkled in here related to property bounds. Currently I have it set up to match the commit interval cluster config, but that doesn't feel quite right. wdyt?

bharathv · 2025-02-07T18:36:56Z

src/v/config/configuration.cc

+      "Default value for the redpanda.iceberg.target.lag.ms topic property",
+      {.needs_restart = needs_restart::no, .visibility = visibility::user},
+      // TODO(oren): better default?
+      std::chrono::milliseconds(1min),


My gut feeling is the default should be a bit higher, at least 5mins or so, just to give flexibility to the scheduler to schedule in more interesting ways rather than just missing deadlines all the time. This needs an empirical evaluation once all the parts are hooked up.

Also purely from a user standpoint my understanding is very few users really want to land data within a minute, my guess is this is typically in hours (I could be wrong here) but the default should be orders of minutes at least.

5mins or so...to give flexibility to the scheduler

makes sense. by the same token, should the minimum value be a couple orders of magnitude higher? not sure how much of a footgun it to allow setting to 10ms, but it doesn't seem like a realistic choice in any case.

yes, I agree.

bharathv · 2025-02-07T18:37:18Z

src/v/config/configuration.cc

+  , iceberg_target_lag_ms(
+      *this,
+      "iceberg_target_lag_ms",
+      "Default value for the redpanda.iceberg.target.lag.ms topic property",


maybe add a little more detail on what the property controls?

bharathv · 2025-02-07T18:41:03Z

src/v/cluster/topic_properties.cc

@@ -150,6 +153,8 @@ bool topic_properties::requires_remote_erase() const {
           && !read_replica.value_or(false) && remote_delete;
 }

+// TODO(oren): need a check somewhere s.t. if iceberg is enabled we populate


I did something like this in the past

Users of this property get ntp_config from the raft->log instance

nice. i think in this case doing nothing is probably fine. I assume we'll still have something like this once your port is done?

redpanda/src/v/datalake/datalake_manager.cc

Lines 297 to 301 in 90c713a

start_translator(

partition,

topic_cfg->properties.iceberg_mode,

topic_cfg->properties.iceberg_invalid_record_action.value_or(

config::shard_local_cfg().iceberg_invalid_record_action.value()));

then just wire the topic config in there?

yes exactly.

src/v/kafka/server/handlers/configs/config_response_utils.cc

src/v/cluster/types.cc

bharathv · 2025-02-07T18:47:33Z

tests/rptest/tests/describe_topics_test.py

+            ConfigProperty(
+                config_type="LONG",
+                value="10",
+                doc_string="Something just tell me by failing",


lol. and of course it didn't fail, so I forgot about it!

:D, adding new properties is surprisingly complicated, so many places to update.

oh, i think it's even a bit weirder than that. At least half of the ConfigPropertys here never appear in the describe response as formulated, so they are just dead code afaict.

Looks to be a case of many properties not appearing in describe responses unless explicitly requested? I'll see whether I can fix it up async with this diff.

vbotbuildovich · 2025-02-07T20:05:19Z

Retry command for Build#61737

please wait until all jobs are finished before running the slash command


/ci-repeat 1
tests/rptest/tests/cluster_config_test.py::ClusterConfigTest.test_valid_settings

bharathv

added @mattschumpert to the PR to sign-off on the config/property names and defaults.

bharathv · 2025-02-07T20:47:25Z

src/v/config/configuration.cc

+      "Default value for the redpanda.iceberg.target.lag.ms topic property",
+      {.needs_restart = needs_restart::no, .visibility = visibility::user},
+      // TODO(oren): better default?
+      std::chrono::milliseconds(1min),


yes, I agree.

bharathv · 2025-02-07T20:49:17Z

tests/rptest/tests/describe_topics_test.py

+            ConfigProperty(
+                config_type="LONG",
+                value="10",
+                doc_string="Something just tell me by failing",


:D, adding new properties is surprisingly complicated, so many places to update.

oleiman · 2025-02-07T21:34:26Z

force push CR comments

bharathv

lgtm, bunch of nits.

src/v/config/configuration.cc

bharathv · 2025-02-10T16:39:12Z

src/v/config/configuration.cc

+      "effor fashion, subject to resource availability.",
+      {.needs_restart = needs_restart::no, .visibility = visibility::user},
+      std::chrono::milliseconds(5min),
+      {.min = 10ms, .max = serde::max_serializable_ms})


bump the min default too?

yeah I was going to see whether @mattschumpert would chime in, but I'll just stick it at like 10s and we can change if needed.

bharathv · 2025-02-10T16:39:27Z

src/v/cluster/topic_properties.cc

@@ -150,6 +153,8 @@ bool topic_properties::requires_remote_erase() const {
           && !read_replica.value_or(false) && remote_delete;
 }

+// TODO(oren): need a check somewhere s.t. if iceberg is enabled we populate


yes exactly.

bharathv · 2025-02-10T16:40:03Z

src/v/kafka/server/handlers/topics/validators.h

+            try {
+                auto val = boost::lexical_cast<std::chrono::milliseconds::rep>(
+                  it->value.value());
+                return val >= 10 && val <= serde::max_serializable_ms.count();


same, bump the default min?

Default value for the corresponding topic property: 'redpanda.iceberg.target.lag.ms'

oleiman · 2025-02-10T22:22:06Z

CI Failure: bazel build raft_reconfiguration_test timeout, unrelated

oleiman self-assigned this Feb 6, 2025

github-actions bot added the area/redpanda label Feb 6, 2025

oleiman force-pushed the dlib/core-8928/target-lag-topic-prop branch from ddb8dc0 to be5cd6a Compare February 7, 2025 00:11

oleiman changed the title ~~cluster: iceberg_target_lag_ms topic property~~ CORE-8928: Introduce redpanda.iceberg.target.lag.ms topic propery Feb 7, 2025

oleiman force-pushed the dlib/core-8928/target-lag-topic-prop branch from be5cd6a to 6b6ed8e Compare February 7, 2025 00:35

oleiman force-pushed the dlib/core-8928/target-lag-topic-prop branch from 6b6ed8e to 22a9bb3 Compare February 7, 2025 03:32

oleiman force-pushed the dlib/core-8928/target-lag-topic-prop branch from 22a9bb3 to d6f0245 Compare February 7, 2025 06:04

oleiman force-pushed the dlib/core-8928/target-lag-topic-prop branch from d6f0245 to c50d858 Compare February 7, 2025 16:59

oleiman requested a review from bharathv February 7, 2025 17:01

oleiman marked this pull request as ready for review February 7, 2025 17:01

oleiman requested a review from a team as a code owner February 7, 2025 17:01

bharathv reviewed Feb 7, 2025

View reviewed changes

bharathv requested a review from mattschumpert February 7, 2025 18:55

bharathv reviewed Feb 7, 2025

View reviewed changes

oleiman force-pushed the dlib/core-8928/target-lag-topic-prop branch from c50d858 to c1f6297 Compare February 7, 2025 21:31

bharathv reviewed Feb 10, 2025

View reviewed changes

oleiman added 2 commits February 10, 2025 09:49

config: Introduce iceberg_target_lag_ms

06f2b44

Default value for the corresponding topic property: 'redpanda.iceberg.target.lag.ms'

cluster: Introduce iceberg_target_lag_ms topic property

b6b8d40

oleiman force-pushed the dlib/core-8928/target-lag-topic-prop branch from c1f6297 to b6b8d40 Compare February 10, 2025 17:52

oleiman requested a review from bharathv February 10, 2025 17:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CORE-8928: Introduce redpanda.iceberg.target.lag.ms topic propery #25056

CORE-8928: Introduce redpanda.iceberg.target.lag.ms topic propery #25056

oleiman commented Feb 6, 2025

oleiman commented Feb 6, 2025

oleiman commented Feb 7, 2025

oleiman commented Feb 7, 2025

oleiman commented Feb 7, 2025

vbotbuildovich commented Feb 7, 2025 •

edited

Loading

vbotbuildovich commented Feb 7, 2025 •

edited

Loading

oleiman commented Feb 7, 2025

bharathv Feb 7, 2025

oleiman Feb 7, 2025

bharathv Feb 7, 2025

bharathv Feb 7, 2025

bharathv Feb 7, 2025

oleiman Feb 7, 2025

bharathv Feb 10, 2025

bharathv Feb 7, 2025

oleiman Feb 7, 2025

bharathv Feb 7, 2025

oleiman Feb 7, 2025

vbotbuildovich commented Feb 7, 2025 •

edited

Loading

bharathv left a comment

bharathv Feb 7, 2025

bharathv Feb 7, 2025

oleiman commented Feb 7, 2025

bharathv left a comment

bharathv Feb 10, 2025

oleiman Feb 10, 2025

bharathv Feb 10, 2025

bharathv Feb 10, 2025

oleiman commented Feb 10, 2025

	start_translator(
	partition,
	topic_cfg->properties.iceberg_mode,
	topic_cfg->properties.iceberg_invalid_record_action.value_or(
	config::shard_local_cfg().iceberg_invalid_record_action.value()));

CORE-8928: Introduce redpanda.iceberg.target.lag.ms topic propery #25056

Are you sure you want to change the base?

CORE-8928: Introduce redpanda.iceberg.target.lag.ms topic propery #25056

Conversation

oleiman commented Feb 6, 2025

Backports Required

Release Notes

Improvements

oleiman commented Feb 6, 2025

oleiman commented Feb 7, 2025

oleiman commented Feb 7, 2025

oleiman commented Feb 7, 2025

vbotbuildovich commented Feb 7, 2025 • edited Loading

Retry command for Build#61716

vbotbuildovich commented Feb 7, 2025 • edited Loading

CI test results

oleiman commented Feb 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbotbuildovich commented Feb 7, 2025 • edited Loading

Retry command for Build#61737

bharathv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleiman commented Feb 7, 2025

bharathv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleiman commented Feb 10, 2025

vbotbuildovich commented Feb 7, 2025 •

edited

Loading

vbotbuildovich commented Feb 7, 2025 •

edited

Loading

vbotbuildovich commented Feb 7, 2025 •

edited

Loading