Topic props #1337
Topic props #1337
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds a new documentation page describing per-topic property configuration and verification workflows. Substantially rewrites the topic-properties reference: reorganizes property groups, renames/aliases many properties, introduces Tiered Storage, Remote Read Replicas, and Iceberg-related properties, updates defaults and descriptions, and adjusts cross-references. Also adds a navigation entry for the new how-to page. No code or API surface changes. Sequence Diagram(s)(No sequence diagram generated — changes are documentation-only and do not modify control flow or runtime interactions.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for redpanda-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
modules/reference/pages/properties/topic-properties.adoc (3)
293-295: Normalize xref paths for cluster propertiesSome sections use
xref:reference:properties/cluster-properties.adoc#...while others usexref:./cluster-properties.adoc#.... For consistency and maintainability, use one style across the page. Since this file lives alongsidecluster-properties.adoc, prefer the relative./cluster-properties.adocform.-xref:reference:properties/cluster-properties.adoc#kafka_batch_max_bytes[`kafka_batch_max_bytes`] +xref:./cluster-properties.adoc#kafka_batch_max_bytes[`kafka_batch_max_bytes`] -xref:reference:properties/cluster-properties.adoc#log_segment_size[`log_segment_size`] +xref:./cluster-properties.adoc#log_segment_size[`log_segment_size`]Also applies to: 334-336
166-169: Minor consistency nits: number formatting in Accepted values and Default blocksSome sections wrap numbers in backticks while others don’t; some use asterisks inconsistently around “Default”. Consider normalizing these for a consistent reading experience. Not blocking.
Also applies to: 182-187, 222-229, 244-251, 358-366, 380-387, 398-406, 578-586, 596-603
411-420: Leaders preference: consider surfacing the format constraintsIf there are specific constraints on
racks:<rack1>[,<rack2>,...](e.g., max length, allowed characters, case-sensitivity), briefly document them or link to a constraints section to reduce user error.Also applies to: 423-426
modules/manage/pages/cluster-maintenance/topic-property-configuration.adoc (2)
121-139: Kubernetes Topic resource example: consider adding the CRD version noteIf multiple CRD versions exist in the field, a brief note that
v1alpha2is current (or linking to a compatibility matrix) would prevent user mismatches.
9-12: Redundant “takes effect immediately” note across pagesThis repeats the same note from the reference page. Consider removing here or replacing with a short cross-reference to keep one source of truth. Not blocking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
modules/manage/pages/cluster-maintenance/topic-property-configuration.adoc(1 hunks)modules/reference/pages/properties/topic-properties.adoc(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Feediver1
PR: redpanda-data/docs#1153
File: modules/reference/pages/properties/topic-properties.adoc:45-50
Timestamp: 2025-07-16T19:33:20.420Z
Learning: In the Redpanda documentation, topic property cross-references like <<max.compaction.lag.ms>> and <<min.compaction.lag.ms>> require corresponding property definition sections with anchors like [[maxcompactionlagms]] and [[mincompactionlagms]] to prevent broken links.
📚 Learning: 2025-07-16T19:33:20.420Z
Learnt from: Feediver1
PR: redpanda-data/docs#1153
File: modules/reference/pages/properties/topic-properties.adoc:45-50
Timestamp: 2025-07-16T19:33:20.420Z
Learning: In the Redpanda documentation, topic property cross-references like <<max.compaction.lag.ms>> and <<min.compaction.lag.ms>> require corresponding property definition sections with anchors like [[maxcompactionlagms]] and [[mincompactionlagms]] to prevent broken links.
Applied to files:
modules/reference/pages/properties/topic-properties.adocmodules/manage/pages/cluster-maintenance/topic-property-configuration.adoc
🔇 Additional comments (4)
modules/reference/pages/properties/topic-properties.adoc (1)
216-226: Docs: state that -1 disables per-topic retention (size & time)Verified in-tree: Redpanda uses -1 as a sentinel to mean “disabled / infinite” for topic retention. Update the docs to list -1 and explain its semantics.
Evidence (select locations to review):
- src/v/storage/ntp_config.h — "Handle the special "-1" case." (retention_bytes / retention_time handling).
- src/v/storage/disk_log_impl.cc — comment: "retention bytes has a built-in mechanism (ie -1 / nullopt) to represent infinite retention."
- src/v/kafka/server/handlers/configs/config_response_utils.cc — formats retention values with value_or(-1 / -1ms).
- src/v/kafka/server/handlers/topics/types.cc — topic parsing uses get_tristate_value(...) for retention_bytes / retention_duration.
- src/v/kafka/server/tests/types_conversion_tests.cc — test asserts retention_bytes.is_disabled() true.
Updated wording (apply to both places noted in the original comment):
-*Accepted values:* [1, 9223372036854775807] bytes +*Accepted values:* -1 (disable) or [1, 9223372036854775807] bytes +NOTE: `-1` disables size-based retention for the topic (infinite retention). -*Accepted values:* [1, 9223372036854775] milliseconds +*Accepted values:* -1 (disable) or [1, 9223372036854775] milliseconds +NOTE: `-1` disables time-based retention for the topic (infinite retention).Also apply the same change to the other doc block you flagged (lines 238–246).
modules/manage/pages/cluster-maintenance/topic-property-configuration.adoc (3)
24-30: Good clarity on DEFAULT_CONFIG vs DYNAMIC_TOPIC_CONFIGClear guidance to interpret
rpk topic describesources helps reduce confusion when verifying overrides. Nice.
73-85: Tiered Storage “create” example looks correct and helpfulCreating with
redpanda.remote.recovery,redpanda.remote.write, andredpanda.remote.readat topic creation is consistent with the constraint that recovery is only configurable at creation time. Good inclusion.
160-171: Compression guidance vs reference behaviorThis page says “Configure compression for stored data,” but the reference clarifies
compression.typeis informational and ignored (broker behaves likeproducer). Align the phrasing here to avoid implying the broker enforces compression at topic level.-- Configure compression for stored data (xref:reference:properties/topic-properties.adoc#compressiontype[`compression.type`]). +- Configure producer-side compression behavior (broker stores data as produced; topic-level `compression.type` is informational). See xref:reference:properties/topic-properties.adoc#compressiontype[`compression.type`].Likely an incorrect or invalid review comment.
modules/manage/pages/cluster-maintenance/topic-property-configuration.adoc
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/reference/pages/properties/topic-properties.adoc (1)
1-686: Fix broken cross-references in topic-properties.adocOur verification shows that several
xref:anchors inmodules/reference/pages/properties/topic-properties.adocdon’t exist (neither as explicit[[anchor]]nor as auto-generated from a heading) inmodules/reference/pages/properties/cluster-properties.adoc. You’ll need to either add the missing anchors incluster-properties.adocor update thexref:targets intopic-properties.adocto the correct names:• compaction_strategy
– no heading or explicit anchor in cluster-properties.adoc; add[[compaction_strategy]]before the compaction strategy default section, or adjust to the actual cluster setting name if it differs.• default_topic_replication
– cluster-properties defines “default_topic_replications” (plural); change toxref:…#default_topic_replications.• flush_bytes
– missing; either add[[flush_bytes]]or point at the correct fsync‐bytes setting (e.g.datalake_translator_flush_bytesif that was intended).• flush_ms
– missing; add[[flush_ms]]or correct to the actual delay‐ms section.• initial_retention_local_target_bytes
– cluster file hasinitial_retention_local_target_bytes_default; either add the non-default anchor or update to#initial_retention_local_target_bytes_default.• initial_retention_local_target_ms
– same as above: use#initial_retention_local_target_ms_defaultor add an anchor.• replication_factor
– cluster file listsinternal_topic_replication_factoranddefault_topic_replications; pick the proper target (likelyinternal_topic_replication_factor) and update to#internal_topic_replication_factor.• retention_local_target_bytes
– cluster file only hasretention_local_target_bytes_default; update to#retention_local_target_bytes_defaultor add[[retention_local_target_bytes]].• retention_local_target_ms
– same: use#retention_local_target_ms_defaultor add the non-default anchor.After making these changes, re-run the anchor-verification script to ensure zero missing anchors.
♻️ Duplicate comments (1)
modules/reference/pages/properties/topic-properties.adoc (1)
21-79: Thanks for addressing prior review itemsPrevious issues called out in earlier review rounds have been resolved:
- Correct cluster mappings for
delete.retention.ms,retention.ms,segment.ms, andwrite.caching.- Default and behavior for
compression.typeclarified.- Fixed xref fragments and Iceberg link syntax.
Also applies to: 244-250, 274-278, 444-449, 680-686
🧹 Nitpick comments (4)
modules/reference/pages/properties/topic-properties.adoc (4)
11-11: Qualify the “takes effect immediately” noteSome properties are create-time-only (for example,
redpanda.remote.recovery). Suggest qualifying the blanket statement.-NOTE: All topic properties take effect immediately after being set. +NOTE: Unless otherwise noted, all topic properties take effect immediately after being set.
167-167: Fix bold formatting on Default linesMinor formatting glitch: closing asterisk is misplaced.
-**Default:* `9223372036854775` +**Default**: `9223372036854775`-**Default:* `0` +**Default**: `0`Also applies to: 203-203
269-270: Clarify “override” statement for compression.typeAbove, you note the broker ignores non-
producervalues and uses producer semantics. Saying it “overrides the cluster property” can read as contradictory. Suggest clarifying that the mapping exists for compatibility and that onlyproduceris effective.-When set, this property overrides the cluster property xref:./cluster-properties.adoc#log_compression_type[`log_compression_type`] for the topic. +Setting this property has no effect beyond the default producer semantics; the only effective value is `producer`. The mapping to xref:./cluster-properties.adoc#log_compression_type[`log_compression_type`] exists for Kafka compatibility.
1-686: Optional: standardize local xref style for cluster-propertiesWithin this page, some links use
xref:cluster-properties.adoc#...while others usexref:./cluster-properties.adoc#.... Both resolve, but consistency simplifies maintenance. Consider standardizing on one form (prefer the relative./).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
modules/ROOT/nav.adoc(1 hunks)modules/manage/pages/cluster-maintenance/topic-property-configuration.adoc(1 hunks)modules/reference/pages/properties/topic-properties.adoc(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/manage/pages/cluster-maintenance/topic-property-configuration.adoc
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-16T19:33:20.420Z
Learnt from: Feediver1
PR: redpanda-data/docs#1153
File: modules/reference/pages/properties/topic-properties.adoc:45-50
Timestamp: 2025-07-16T19:33:20.420Z
Learning: In the Redpanda documentation, topic property cross-references like <<max.compaction.lag.ms>> and <<min.compaction.lag.ms>> require corresponding property definition sections with anchors like [[maxcompactionlagms]] and [[mincompactionlagms]] to prevent broken links.
Applied to files:
modules/ROOT/nav.adocmodules/reference/pages/properties/topic-properties.adoc
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - redpanda-docs-preview
- GitHub Check: Header rules - redpanda-docs-preview
- GitHub Check: Pages changed - redpanda-docs-preview
🔇 Additional comments (1)
modules/ROOT/nav.adoc (1)
153-154: Verified & Approved:topic-property-configurationxrefI’ve confirmed that:
- The target file exists at
modules/manage/pages/cluster-maintenance/topic-property-configuration.adoc- It declares a top-level title (
= Configure Topic Properties), so the AsciiDoc xref will resolve- Other modules reference it correctly (e.g., in
modules/reference/pages/properties/topic-properties.adoc)Everything looks good—merging now.
# Conflicts: # modules/reference/pages/properties/topic-properties.adoc
| Configure properties to manage disk space usage for a topic: | ||
|
|
||
| - Clean up log segments by deletion and/or compaction (xref:reference:properties/topic-properties.adoc#cleanuppolicy[`cleanup.policy`]). | ||
| - Control compaction timing with maximum and minimum lag settings (xref:reference:properties/topic-properties.adoc#maxcompactionlagms[`max.compaction.lag.ms`] and xref:reference:properties/topic-properties.adoc#mincompactionlagms[`min.compaction.lag.ms`]). |
There was a problem hiding this comment.
may want to also mention min.cleanable.dirty.ratio here, since those are the 3 major tweakables for compaction at the topic level.
| - Recover data from object storage (xref:reference:properties/topic-properties.adoc#redpandaremoterecovery[`redpanda.remote.recovery`]). | ||
| - Configure local storage retention limits (xref:reference:properties/topic-properties.adoc#retentionlocaltargetbytes[`retention.local.target.bytes`] and xref:reference:properties/topic-properties.adoc#retentionlocaltargetms[`retention.local.target.ms`]). | ||
|
|
||
| === Compaction settings |
There was a problem hiding this comment.
Kind of awkward to have this here when min.compaction.lag.ms and max.compaction.lag.ms were discussed above
| Local:: | ||
| + | ||
| -- | ||
| include::develop:partial$topic-properties-warning.adoc[] |
There was a problem hiding this comment.
Should this warning be elsewhere in the document? placement looks a little weird in the preview
|
|
||
| == Verify topic property configuration | ||
|
|
||
| The `SOURCE` output of the xref:reference:rpk/rpk-topic/rpk-topic-describe.adoc[`rpk topic describe <topic>`] command shows how properties are configured for the topic: |
There was a problem hiding this comment.
This section might benefit from showing an example output of rpk topic describe and pointing out the DEFAULT_CONFIG vs DYNAMIC_TOPIC_CONFIG with it instead of just describing what the output will look like when ran.
| * `DEFAULT_CONFIG` is set by a Redpanda default. | ||
| * `DYNAMIC_TOPIC_CONFIG` is set by the user specifically for the topic and overrides inherited default configurations, such as a default or a cluster-level property. | ||
|
|
||
| Although xref:reference:rpk/rpk-topic/rpk-topic-describe.adoc[`rpk topic describe`] doesn't report `replication.factor` as a configuration, `replication.factor` can indeed be set by using the xref:reference:rpk/rpk-topic/rpk-topic-alter-config.adoc[`rpk topic alter-config`] command. |
There was a problem hiding this comment.
rpk topic describe does output a summary like
SUMMARY
=======
NAME {topic}
PARTITIONS 1
REPLICAS 3
which indicates the replication.factor.
You may also want to add a link/disclaimer that more involved information about a topic, its partitions & replicas can be viewed with rpk topic describe {topic} -p.
Description
Heavily reorganize the topic properties documentation.
Split reference to action.
Increase guidelines on the action.
Reorganize reference, adding new groups, adding background information, evaluating types, defaults and acceptable ranges.
Introduce new topic properties:
Related new automation
redpanda-data/docs-extensions-and-macros#126
Resolves https://redpandadata.atlassian.net/browse/DOC-145
Review deadline: Sept 1st 2025
Page previews
Topic property reference
Topic property configuration
Diff checker
Github can be incovenient for this PR, as the liner numbers have diverged greatly. If you want to compare this version with previous, check latest content on main https://raw.githubusercontent.com/redpanda-data/docs/refs/heads/main/modules/reference/pages/properties/topic-properties.adoc
Checks