-
Notifications
You must be signed in to change notification settings - Fork 47
Update topic prop #1527
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
base: main
Are you sure you want to change the base?
Update topic prop #1527
Conversation
…property-overrides
✅ Deploy Preview for redpanda-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
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 📝 WalkthroughWalkthroughThe pull request updates the documentation update workflow to trigger on pushes to docs-data/property-overrides.json, uses a dynamic checkout ref (current branch on push, main otherwise), and extends tag resolution from antora.yml for push events. It implements dual-path regeneration checks: for push events, compares file mtimes of docs-data/property-overrides.json and the generated broker properties doc; for non-push events, retains semver tag comparison. When regeneration is needed on a push, generated docs are committed back to the branch; when needed for non-push events, a PR is created. The property-overrides.json and topic property docs were expanded to document a three-state model (positive, 0, negative) across many retention, segment, and audit properties. Sequence DiagramsequenceDiagram
actor GitHub as GitHub Event
participant Workflow as GH Actions Workflow
participant Repo as Repository (checkout)
participant Gen as Doc Generator (doc-tools/antora)
participant PR as PR Creator
GitHub->>Workflow: Trigger (push or non-push)
Workflow->>Workflow: Determine ref & tag (branch for push, main/tag for others)
alt Push Event
Workflow->>Repo: Checkout current branch
Workflow->>Workflow: Compare mtime: docs-data/property-overrides.json vs modules/.../broker-properties.adoc
Workflow->>Workflow: is_newer = (override newer OR generated missing)
else Non-Push Event
Workflow->>Repo: Checkout main
Workflow->>Workflow: Compare incoming tag vs antora.yml latest-redpanda-tag (semver)
Workflow->>Workflow: is_newer = (incoming tag newer)
end
alt is_newer == true
Workflow->>Gen: Run doc generation (with overrides path)
Gen->>Repo: Produce/modify broker-properties.adoc
alt Push Event
Workflow->>Repo: git config, stage files
Workflow->>Repo: Commit & push changes to current branch
else Non-Push Event
Workflow->>PR: Create branch and open PR with generated docs (branch includes tag)
end
else is_newer == false
Workflow->>Workflow: Log "No regeneration needed" and exit
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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. Comment |
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.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs-data/property-overrides.json (3)
326-326: Remove unclear "Kafka protocol behavior" reference.The phrase "following Kafka protocol behavior" doesn't apply to Redpanda-specific features like object storage housekeeping. Consider removing this phrase or replacing it with clearer language about what the default behavior actually is.
🔎 Suggested clarification
- "description": "Interval between object storage housekeeping tasks.\n\nThis property supports three states:\n\n* Positive value: Sets the interval in milliseconds between housekeeping operations. Smaller values provide more frequent cleanup but increase CPU and I/O usage.\n* 0: Disables automatic housekeeping operations. Manual cleanup may be required.\n* Negative value: Uses the default system interval for housekeeping tasks, following Kafka protocol behavior." + "description": "Interval between object storage housekeeping tasks.\n\nThis property supports three states:\n\n* Positive value: Sets the interval in milliseconds between housekeeping operations. Smaller values provide more frequent cleanup but increase CPU and I/O usage.\n* 0: Disables automatic housekeeping operations. Manual cleanup may be required.\n* Negative value: Uses the cluster-wide default interval for housekeeping tasks."
1689-1720: Consistent but potentially confusing "disabled" terminology across retention properties.All four retention properties (retention.bytes, retention.local.target.bytes, retention.local.target.ms, retention.ms) use the same pattern for describing negative values: "disabled by setting to a negative value (no per-topic limit applied)".
While consistent, the term "disabled" is potentially confusing because:
- In retention contexts, "disabled" typically means data is never cleaned up (infinite retention)
- Here it means "no per-topic limit", which actually could result in more aggressive cleanup if there's a cluster-wide default
Consider using clearer terminology like "set to a negative value (removes per-topic limit, cluster-wide retention applies)" or similar.
This is consistent across all retention properties, so if you decide to change it, update all four for consistency.
Based on learnings about keeping descriptions focused on behavior and explaining important behavioral details.
1752-1752: Remove unclear "Kafka protocol behavior" reference.Similar to
cloud_storage_housekeeping_interval_msandaudit_enabled, this property includes the phrase "following Kafka protocol behavior" which doesn't add clarity for Redpanda-specific retention behavior. Consider removing or replacing with clearer language.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/update-property-docs.yaml(4 hunks)docs-data/property-overrides.json(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs-data/property-overrides.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
docs-data/property-overrides.json: Update/docs-data/property-overrides.jsonto make changes to property documentation - this is the file that drives auto-generation of property reference pages
Always read the override file first before making updates to preserve existing overrides
Validate JSON syntax after updating property-overrides.json using: python -c "import json; json.load(open('docs-data/property-overrides.json'))"
In property descriptions, never add enterprise license includes or markers likeinclude::reference:partial$enterprise-licensed-property.adoc[]
In property descriptions, never add cloud-specific conditional blocks or notes about BYOC, Dedicated, or read-only status
Never add or update descriptions for properties marked as deprecated - remove any existing override for deprecated properties
Always use full Antora resource IDs with module prefixes in xref links within property descriptions (e.g.,reference:properties/cluster-properties.adoc, never./cluster-properties.adoc)
Prefix self-managed-only links withself-managed-only:in related_topics to handle documentation pages that only exist in self-managed deployments
Remove duplicate links from related_topics lists in property overrides to keep them clean
In property descriptions, keep descriptions focused on behavior and avoid metadata information like version availability, cloud availability, license requirements, requires restart status, default values, and type information
In property descriptions, explain what the property does, when to use it, how it relates to other properties, and important behavioral details
Use AsciiDoc formatting in property descriptions:property_namefor property names,xref:module:path/to/doc.adoc[Link Text]for cross-references, and\n\nfor paragraph breaks
For enum-type properties in overrides, add value explanations showing what each value means
Normalize all xref links in property-overrides.json to use full Antora resource IDs after updating
Files:
docs-data/property-overrides.json
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Update `/docs-data/property-overrides.json` to make changes to property documentation - this is the file that drives auto-generation of property reference pages
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, explain what the property does, when to use it, how it relates to other properties, and important behavioral details
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Normalize all xref links in property-overrides.json to use full Antora resource IDs after updating
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Always read the override file first before making updates to preserve existing overrides
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Never add or update descriptions for properties marked as deprecated - remove any existing override for deprecated properties
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Use AsciiDoc formatting in property descriptions: `property_name` for property names, `xref:module:path/to/doc.adoc[Link Text]` for cross-references, and `\n\n` for paragraph breaks
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Validate JSON syntax after updating property-overrides.json using: python -c "import json; json.load(open('docs-data/property-overrides.json'))"
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : For enum-type properties in overrides, add value explanations showing what each value means
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, never add cloud-specific conditional blocks or notes about BYOC, Dedicated, or read-only status
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, keep descriptions focused on behavior and avoid metadata information like version availability, cloud availability, license requirements, requires restart status, default values, and type information
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Update `/docs-data/property-overrides.json` to make changes to property documentation - this is the file that drives auto-generation of property reference pages
Applied to files:
.github/workflows/update-property-docs.yamldocs-data/property-overrides.json
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Normalize all xref links in property-overrides.json to use full Antora resource IDs after updating
Applied to files:
.github/workflows/update-property-docs.yaml
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, keep descriptions focused on behavior and avoid metadata information like version availability, cloud availability, license requirements, requires restart status, default values, and type information
Applied to files:
docs-data/property-overrides.json
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, explain what the property does, when to use it, how it relates to other properties, and important behavioral details
Applied to files:
docs-data/property-overrides.json
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, never add cloud-specific conditional blocks or notes about BYOC, Dedicated, or read-only status
Applied to files:
docs-data/property-overrides.json
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Never add or update descriptions for properties marked as deprecated - remove any existing override for deprecated properties
Applied to files:
docs-data/property-overrides.json
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : For enum-type properties in overrides, add value explanations showing what each value means
Applied to files:
docs-data/property-overrides.json
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, never add enterprise license includes or markers like `include::reference:partial$enterprise-licensed-property.adoc[]`
Applied to files:
docs-data/property-overrides.json
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Use AsciiDoc formatting in property descriptions: `property_name` for property names, `xref:module:path/to/doc.adoc[Link Text]` for cross-references, and `\n\n` for paragraph breaks
Applied to files:
docs-data/property-overrides.json
📚 Learning: 2025-07-16T19:33:20.420Z
Learnt from: Feediver1
Repo: redpanda-data/docs PR: 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:
docs-data/property-overrides.json
⏰ 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 (11)
.github/workflows/update-property-docs.yaml (7)
13-14: LGTM! Push trigger correctly configured.The push event trigger for
docs-data/property-overrides.jsonis properly configured and will automatically regenerate documentation when the overrides file is modified.
41-41: LGTM! Dynamic branch selection is correct.The conditional branch checkout properly handles both push events (using the actual push ref) and non-push events (defaulting to main).
59-63: Consider adding validation for the extracted tag.The tag extraction from
antora.ymlworks correctly, but there's no validation if the extraction fails or returns an empty value. Consider adding a check after line 62:🔎 Suggested enhancement for tag validation
elif [ "${{ github.event_name }}" = "push" ]; then # For push events, use the current latest-redpanda-tag from antora.yml CURRENT=$(grep 'latest-redpanda-tag:' antora.yml | awk '{print $2}' | tr -d "\"'") + if [ -z "$CURRENT" ]; then + echo "❌ Failed to extract latest-redpanda-tag from antora.yml" >&2 + exit 1 + fi echo "tag=$CURRENT" >> $GITHUB_OUTPUT echo "🔄 Using current latest-redpanda-tag for override changes: $CURRENT"
82-101: LGTM! File modification time comparison logic is sound.The dual-path regeneration check correctly handles both scenarios:
- For push events: compares file modification times to determine if regeneration is needed
- Properly handles the case where generated docs don't exist
The logic flow is correct and will prevent unnecessary regeneration when the generated docs are already up-to-date.
103-114: LGTM! Semver comparison logic is correct.The version comparison for non-push events properly:
- Strips version prefixes for numeric comparison
- Uses
sort -Vfor semantic version ordering- Ensures the new tag is actually newer before triggering regeneration
130-145: LGTM! Git commit and push logic is well-structured.The commit step correctly:
- Runs only for push events when regeneration is needed
- Configures git identity appropriately
- Checks for actual changes before committing (prevents empty commits)
- Uses clear commit message with "auto-docs:" prefix for easy identification
148-153: LGTM! PR creation logic correctly excludes push events.The conditional ensures:
- PRs are only created for workflow_dispatch and repository_dispatch events
- Push events commit directly to the branch (as handled in the previous step)
- Branch names include the tag for clear identification
This design allows for automatic in-branch regeneration when overrides change, while maintaining PR-based flow for version updates.
docs-data/property-overrides.json (4)
956-970: LGTM! Initial retention properties are well-documented.Both
initial.retention.local.target.bytesandinitial.retention.local.target.mshave clear, consistent descriptions that:
- Explain the purpose (controlling data transfer during cluster resizing)
- Define three-state behavior clearly
- Include proper cross-references to related cluster properties
- Follow the documentation guidelines
1928-1936: Comprehensive description with minor terminology issue.The description for
segment.msis thorough and includes valuable information about:
- How log segment rolling works
- Three-state behavior
- Value clamping between min/max bounds
- How to check current bounds with rpk
However, it uses the same "disabled by setting to a negative value" terminology that appears in other properties, which could be clearer (see previous comments about retention properties).
Overall, this is a well-documented property with good behavioral details and practical guidance.
As per coding guidelines about explaining what properties do, when to use them, and important behavioral details.
1909-1914: LGTM! Comprehensive cluster bootstrap guidance.The expanded description for
seed_serversprovides excellent behavioral details:
- Clear explanation of how seed_servers works with empty_seed_starts_cluster
- Separate guidance for both true and false scenarios
- Important warnings about cluster formation
- Detailed examples with placeholder explanations
This follows the coding guidelines to explain what the property does, when to use it, how it relates to other properties, and important behavioral details.
Based on learnings about explaining relationships with other properties and important behavioral details.
1-2081: JSON syntax is valid. No errors detected in property-overrides.json.
d163039 to
54de33e
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
modules/reference/partials/properties/topic-properties.adoc (7)
53-53: Duplicate cross-reference with inconsistent formats.Lines 53 and 59 both reference
log_cleanup_policy, but use different xref formats. Line 53 usesxref:cluster-properties.adocwhile line 59 usesxref:reference:properties/cluster-properties.adoc. Per the learnings, always use the full Antora resource ID format. Consider removing the duplicate or consolidating to a single, consistently formatted reference.Proposed fix
| Related topics | * xref:reference:properties/cluster-properties.adoc#log_cleanup_policy[`log_cleanup_policy`] * xref:manage:cluster-maintenance/disk-utilization.adoc#configure-segment-size[Configure segment size] * xref:manage:tiered-storage.adoc#compacted-topics-in-tiered-storage[Compacted topics in Tiered Storage] - * xref:reference:properties/cluster-properties.adoc#log_cleanup_policy[`log_cleanup_policy`] |===Also applies to: 59-59
932-935: Duplicate and inconsistently formatted related topics entries.Lines 932–935 contain two entries for
min_compaction_lag_mswith different xref formats. The first (line 933) usesxref:cluster-properties.adoc, while the second (line 935) usesxref:reference:properties/cluster-properties.adoc. Per the learnings, use the full Antora resource ID consistently, and remove the duplicate.Proposed fix
| Related topics | * xref:reference:properties/cluster-properties.adoc#min_compaction_lag_ms[`min_compaction_lag_ms`] - * xref:cluster-properties.adoc#min_compaction_lag_ms[`min_compaction_lag_ms`] - - * xref:reference:properties/cluster-properties.adoc#min_compaction_lag_ms[`min_compaction_lag_ms`] * xref:manage:cluster-maintenance/compaction-settings.adoc#configure-min-compaction-lag[Configure minimum compaction lag] |===
1764-1764: Inconsistent xref format for retention_bytes references.Lines 1764 and 1766 both reference
retention_bytes, but line 1764 usesxref:cluster-properties.adocwhile line 1766 uses the full resource IDxref:reference:properties/cluster-properties.adoc. Standardize on the full format per documentation guidelines.Proposed fix
| Related topics | - * xref:cluster-properties.adoc#retention_bytes[`retention_bytes`] + * xref:reference:properties/cluster-properties.adoc#retention_bytes[`retention_bytes`] * xref:reference:properties/cluster-properties.adoc#retention_bytes[`retention_bytes`] * xref:manage:cluster-maintenance/disk-utilization.adoc#configure-message-retention[Configure message retention] |===Also applies to: 1766-1766
1939-1940: Inconsistent xref format: use full Antora resource ID.These xrefs use relative path format (
xref:./cluster-properties.adoc), but should use the full Antora resource ID (xref:reference:properties/cluster-properties.adoc). Standardize on the full format per documentation guidelines.Proposed fix
When `segment.bytes` is set to a positive value, it overrides the cluster property: - * xref:./cluster-properties.adoc#log_segment_size[`log_segment_size`] for non-compacted topics - * xref:./cluster-properties.adoc#compacted_log_segment_size[`compacted_log_segment_size`] for compacted topics (when `cleanup.policy=compact`) + * xref:reference:properties/cluster-properties.adoc#log_segment_size[`log_segment_size`] for non-compacted topics + * xref:reference:properties/cluster-properties.adoc#compacted_log_segment_size[`compacted_log_segment_size`] for compacted topics (when `cleanup.policy=compact`)
2099-2100: Duplicate reference with inconsistent format.Lines 2099 and 2100 both reference
write_caching_default. Line 2099 usesxref:cluster-properties.adocwhile line 2100 uses the full resource ID. Remove the duplicate and standardize on the full Antora resource ID format.Proposed fix
* xref:reference:properties/cluster-properties.adoc#write_caching_default[`write_caching_default`] - * xref:cluster-properties.adoc#write_caching_default[`write_caching_default`] |===
112-112: Update fix location: changes must be made in property-overrides.json, not in the generated file.This file is autogenerated from
docs-data/property-overrides.json. The xref at line 112 uses a relative path format (xref:./cluster-properties.adoc), but should use the full Antora resource ID (xref:reference:properties/cluster-properties.adoc) per documentation guidelines. However, xref macros use Antora resource IDs, which are unique identifying coordinates, and this approach eliminates the relative path problem by specifying resources as family-relative directory paths.Apply this fix to the property description in
property-overrides.jsonfor the relevant property:-xref:./cluster-properties.adoc#log_compression_type +xref:reference:properties/cluster-properties.adoc#log_compression_typeDo not modify the generated
.adocfile directly, as changes will be overwritten when the file is regenerated.
659-659: Convert malformedconfig_ref:syntax to proper Antora xref format.Line 659 uses a non-standard
config_ref:macro with comma-separated arguments that violates Antora xref syntax requirements. This should be a standardxreflink with full Antora resource ID.-Set an upper limit for `max.message.bytes` using the cluster property config_ref:kafka_max_message_size_upper_limit_bytes,true,properties/cluster-properties[`kafka_max_message_size_upper_limit_bytes`]. +Set an upper limit for `max.message.bytes` using the cluster property xref:reference:properties/cluster-properties.adoc#kafka_max_message_size_upper_limit_bytes[`kafka_max_message_size_upper_limit_bytes`].This fix should be applied in
docs-data/property-overrides.jsonunder themax.message.bytesproperty description field, then the documentation should be regenerated using the doc-tools CLI.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
modules/reference/partials/properties/topic-properties.adoc(9 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Update `/docs-data/property-overrides.json` to make changes to property documentation - this is the file that drives auto-generation of property reference pages
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Normalize all xref links in property-overrides.json to use full Antora resource IDs after updating
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Always use full Antora resource IDs with module prefixes in xref links within property descriptions (e.g., `reference:properties/cluster-properties.adoc`, never `./cluster-properties.adoc`)
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Prefix self-managed-only links with `self-managed-only:` in related_topics to handle documentation pages that only exist in self-managed deployments
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, keep descriptions focused on behavior and avoid metadata information like version availability, cloud availability, license requirements, requires restart status, default values, and type information
Applied to files:
modules/reference/partials/properties/topic-properties.adoc
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, explain what the property does, when to use it, how it relates to other properties, and important behavioral details
Applied to files:
modules/reference/partials/properties/topic-properties.adoc
📚 Learning: 2025-07-16T19:33:20.420Z
Learnt from: Feediver1
Repo: redpanda-data/docs PR: 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/partials/properties/topic-properties.adoc
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : Use AsciiDoc formatting in property descriptions: `property_name` for property names, `xref:module:path/to/doc.adoc[Link Text]` for cross-references, and `\n\n` for paragraph breaks
Applied to files:
modules/reference/partials/properties/topic-properties.adoc
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, never add enterprise license includes or markers like `include::reference:partial$enterprise-licensed-property.adoc[]`
Applied to files:
modules/reference/partials/properties/topic-properties.adoc
📚 Learning: 2025-11-25T09:42:15.235Z
Learnt from: CR
Repo: redpanda-data/docs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T09:42:15.235Z
Learning: Applies to docs-data/property-overrides.json : In property descriptions, never add cloud-specific conditional blocks or notes about BYOC, Dedicated, or read-only status
Applied to files:
modules/reference/partials/properties/topic-properties.adoc
📚 Learning: 2025-12-12T16:18:28.126Z
Learnt from: andrewstucki
Repo: redpanda-data/docs PR: 1514
File: modules/troubleshoot/partials/errors-and-solutions.adoc:618-632
Timestamp: 2025-12-12T16:18:28.126Z
Learning: In Redpanda ShadowLink CRD usage, do not copy CA certificates into pods via kubectl cp because they won't persist across pod restarts. Store CA certs in Kubernetes Secrets and reference them in the ShadowLink's staticConfiguration block (or inline the cert data in that block) to ensure persistence and proper secret management.
Applied to files:
modules/reference/partials/properties/topic-properties.adoc
🔇 Additional comments (1)
modules/reference/partials/properties/topic-properties.adoc (1)
320-324: Excellent: three-state model descriptions are clear and consistent.The expanded property descriptions across all updated lines clearly articulate the three-state model semantics (positive, 0, negative) for retention, storage, and segment properties. The explanations of behavior for each state are precise and helpful. This aligns well with the property overrides documentation and provides users with actionable guidance.
Also applies to: 502-508, 555-561, 847-853, 1724-1728, 1777-1783, 1830-1836, 1885-1891, 1993-1999
dbac0ec to
45fd5c1
Compare
topic prop update additional changes revert audit change revert cluster prop change update with links
change admonition
45fd5c1 to
69545dc
Compare
WillemKauf
left a comment
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 think Claude hallucinated a lot here. I reviewed half the PR (half of it seems to reiterate the same information on another docs page) but please review the topic properties edited and make sure the issues I called out are reflected in all altered pages.
docs-data/property-overrides.json
Outdated
| }, | ||
| "delete.retention.ms": { | ||
| "description": "The retention time for tombstone records in a compacted topic. Redpanda removes tombstone records after the retention limit is exceeded.\n\nIf you have enabled Tiered Storage and set <<redpandaremoteread,`redpanda.remote.read`>> or <<redpandaremotewrite,`redpanda.remote.write`>> for the topic, you cannot enable tombstone removal.\n\nIf both `delete.retention.ms` and the cluster property config_ref:tombstone_retention_ms,true,properties/cluster-properties[] are set, `delete.retention.ms` overrides the cluster level tombstone retention for an individual topic.", | ||
| "description": "The retention time for tombstone records in a compacted topic. Redpanda removes tombstone records after the retention limit is exceeded.\n\nIf you have enabled Tiered Storage and set <<redpandaremoteread,`redpanda.remote.read`>> or <<redpandaremotewrite,`redpanda.remote.write`>> for the topic, you cannot enable tombstone removal.\n\nIf both `delete.retention.ms` and the cluster property config_ref:tombstone_retention_ms,true,properties/cluster-properties[] are set, `delete.retention.ms` overrides the cluster level tombstone retention for an individual topic.\n\nThis property supports three states:\n\n* Positive value: Sets the milliseconds to retain tombstone records before removal.\n* 0: Tombstone records are immediately eligible for removal.\n* Negative value: No per-topic limit applied, inherits cluster default from xref:reference:properties/cluster-properties.adoc#tombstone_retention_ms[`tombstone_retention_ms`].", |
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.
Negative value: No per-topic limit applied, inherits cluster default from
Not true- this disables the feature entirely. Anything < 0 leaves this property disabled.
docs-data/property-overrides.json
Outdated
| }, | ||
| "min.cleanable.dirty.ratio": { | ||
| "description": "The minimum ratio between the number of bytes in dirty segments and the total number of bytes in closed segments that must be reached before a partition's log is eligible for compaction in a compact topic.", | ||
| "description": "The minimum ratio between dirty and total bytes in closed segments before a partition's log is eligible for compaction in a compact topic.\n\nThis property supports three states:\n\n* Positive value: Sets the minimum dirty ratio (0.0 to 1.0) required before compaction.\n* 0: Compaction is always eligible regardless of dirty ratio.\n* Negative value: Uses the cluster default from xref:reference:properties/cluster-properties.adoc#min_cleanable_dirty_ratio[`min_cleanable_dirty_ratio`].", |
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.
Negative value: Uses the cluster default from
Incorrect. min.cleanable.dirty.ratio won't be considered when deciding if a log is valid for compaction.
(This property shouldn't support three states- it's unfortunate it was implemented this way.)
docs-data/property-overrides.json
Outdated
| }, | ||
| "retention.bytes": { | ||
| "description": "A size-based retention limit that configures the maximum size that a topic partition can grow before becoming eligible for cleanup.\n\nIf `retention.bytes` is set to a positive value, it overrides the cluster property xref:cluster-properties.adoc#retention_bytes[`retention_bytes`] for the topic, and the total retained size for the topic is `retention.bytes` multiplied by the number of partitions for the topic.\n\nWhen both size-based (`retention.bytes`) and time-based (`retention.ms`) retention limits are set, cleanup occurs when either limit is reached.", | ||
| "description": "A size-based retention limit that configures the maximum size that a topic partition can grow before becoming eligible for cleanup.\n\nIf `retention.bytes` is set to a positive value, it overrides the cluster property xref:cluster-properties.adoc#retention_bytes[`retention_bytes`] for the topic, and the total retained size for the topic is `retention.bytes` multiplied by the number of partitions for the topic.\n\nWhen both size-based (`retention.bytes`) and time-based (`retention.ms`) retention limits are set, cleanup occurs when either limit is reached.\n\nThis property supports three states:\n\n* Positive value: Sets the maximum bytes per partition. When exceeded, oldest data becomes eligible for cleanup.\n* 0: Partitions are immediately eligible for cleanup.\n* Negative value: No per-topic limit applied, inherits cluster default from xref:reference:properties/cluster-properties.adoc#retention_bytes[`retention_bytes`].", |
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.
Negative value: No per-topic limit applied, inherits cluster default from
Incorrect, no size based retention will be enforced in the disabled case.
docs-data/property-overrides.json
Outdated
| }, | ||
| "retention.local.target.bytes": { | ||
| "description": "A size-based retention limit for Tiered Storage that configures the maximum size that a topic partition in local storage can grow before becoming eligible for cleanup. It applies per partition and is equivalent to <<retentionbytes, `retention.bytes`>> without Tiered Storage.", | ||
| "description": "A size-based retention limit for Tiered Storage that configures the maximum size that a topic partition in local storage can grow before becoming eligible for cleanup. It applies per partition and is equivalent to <<retentionbytes, `retention.bytes`>> without Tiered Storage.\n\nThis property supports three states:\n\n* Positive value: Sets the maximum bytes per partition in local storage before cleanup.\n* 0: Data in local storage is immediately eligible for cleanup.\n* Negative value: No per-topic limit applied, inherits cluster default from xref:reference:properties/cluster-properties.adoc#retention_local_target_bytes[`retention_local_target_bytes`].", |
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.
Incorrect, no size based local retention override will be applied in the disabled case.
docs-data/property-overrides.json
Outdated
| }, | ||
| "retention.local.target.ms": { | ||
| "description": "A time-based retention limit for Tiered Storage that sets the maximum duration that a log's segment file for a topic is retained in local storage before it's eligible for cleanup. This property is equivalent to <<retentionms, `retention.ms`>> without Tiered Storage.", | ||
| "description": "A time-based retention limit for Tiered Storage that sets the maximum duration that a log's segment file for a topic is retained in local storage before cleanup.\n\nThis property supports three states:\n\n* Positive value: Sets the maximum milliseconds to retain data in local storage.\n* 0: Data in local storage is immediately eligible for cleanup.\n* Negative value: No per-topic limit applied, inherits cluster default from xref:reference:properties/cluster-properties.adoc#retention_local_target_ms_default[`retention_local_target_ms_default`].", |
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.
Negative value: No per-topic limit applied, inherits cluster default
Incorrect, no time based local retention override will be applied in the disabled case.
This property is equivalent to <<retentionms,
retention.ms>> without Tiered Storage
That was a helpful message, why is it removed?
docs-data/property-overrides.json
Outdated
| }, | ||
| "retention.ms": { | ||
| "description": "A time-based retention limit that configures the maximum duration that a log's segment file for a topic is retained before it becomes eligible to be cleaned up. To consume all data, a consumer of the topic must read from a segment before its `retention.ms` elapses, otherwise the segment may be compacted and/or deleted. If a non-positive value, no per-topic limit is applied.\n\nIf `retention.ms` is set to a positive value, it overrides the cluster property xref:./cluster-properties.adoc#log_retention_ms[`log_retention_ms`] for the topic.\n\nWhen both size-based (`retention.bytes`) and time-based (`retention.ms`) retention limits are set, the earliest occurring limit applies.", | ||
| "description": "A time-based retention limit that configures the maximum duration that a log's segment file for a topic is retained before it becomes eligible to be cleaned up. To consume all data, a consumer of the topic must read from a segment before its `retention.ms` elapses, otherwise the segment may be compacted and/or deleted.\n\nIf `retention.ms` is set to a positive value, it overrides the cluster property xref:./cluster-properties.adoc#log_retention_ms[`log_retention_ms`] for the topic.\n\nWhen both size-based (`retention.bytes`) and time-based (`retention.ms`) retention limits are set, the earliest occurring limit applies.\n\nThis property supports three states:\n\n* Positive value: Sets the maximum milliseconds to retain data. After this duration, segments become eligible for cleanup.\n* 0: Data is immediately eligible for cleanup.\n* Negative value: No per-topic limit applied, inherits cluster default from xref:reference:properties/cluster-properties.adoc#log_retention_ms[`log_retention_ms`].", |
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.
Negative value: No per-topic limit applied, inherits cluster default from
Incorrect, no time based limit is enforced in the disabled case.
docs-data/property-overrides.json
Outdated
| }, | ||
| "segment.ms": { | ||
| "description": "The maximum duration that a log segment of a topic is active (open for writes and not deletable). A periodic event, with `segment.ms` as its period, forcibly closes the active segment and transitions, or rolls, to a new active segment. The closed (inactive) segment is then eligible to be cleaned up according to cleanup and retention properties.\n\nIf set to a positive duration, `segment.ms` overrides the cluster property xref:./cluster-properties.adoc#log_segment_ms[`log_segment_ms`]. Values are automatically clamped between the cluster bounds set by xref:./cluster-properties.adoc#log_segment_ms_min[`log_segment_ms_min`] (default: 10 minutes) and xref:./cluster-properties.adoc#log_segment_ms_max[`log_segment_ms_max`] (default: 1 year). If your configured value exceeds these bounds, Redpanda uses the bound value and logs a warning. Check current cluster bounds with `rpk cluster config get log_segment_ms_min log_segment_ms_max`.", | ||
| "description": "The maximum duration that a log segment of a topic is active (open for writes and not deletable). A periodic event, with `segment.ms` as its period, forcibly closes the active segment and transitions, or rolls, to a new active segment. The closed (inactive) segment is then eligible to be cleaned up according to cleanup and retention properties.\n\nIf set to a positive duration, `segment.ms` overrides the cluster property xref:./cluster-properties.adoc#log_segment_ms[`log_segment_ms`]. Values are automatically clamped between the cluster bounds set by xref:./cluster-properties.adoc#log_segment_ms_min[`log_segment_ms_min`] (default: 10 minutes) and xref:./cluster-properties.adoc#log_segment_ms_max[`log_segment_ms_max`] (default: 1 year). If your configured value exceeds these bounds, Redpanda uses the bound value and logs a warning. Check current cluster bounds with `rpk cluster config get log_segment_ms_min log_segment_ms_max`.\n\nThis property supports three states:\n\n* Positive value: Sets the maximum milliseconds a segment remains active before rolling to a new segment.\n* 0: Segments are immediately eligible for closure.\n* Negative value: Uses the cluster default from xref:reference:properties/cluster-properties.adoc#log_segment_ms[`log_segment_ms`].", |
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.
Negative value: Uses the cluster default from
Incorrect, no time based rolling is enforced in the disabled case.
Might also be worth mentioning that max.compaction.lag.ms is also applied as a limit to segment.ms for compact enabled topics here.
|
|
||
| * Positive value: Sets the milliseconds to retain tombstone records before removal. | ||
| * 0: Tombstone records are immediately eligible for removal. | ||
| * Negative value: No per-topic limit applied, inherits cluster default from xref:reference:properties/cluster-properties.adoc#tombstone_retention_ms[`tombstone_retention_ms`]. |
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.
No per-topic limit applied, inherits cluster default from
Incorrect, the feature is disabled and tombstones are never removed in the disabled case.
e57dc66 to
8cf5b24
Compare
WillemKauf
left a comment
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.
This looks good to me!
Description
Resolves https://redpandadata.atlassian.net/browse/DOC-1877
Review deadline: Jan 5th
Updates docs for tristate behavior for topic properties in Redpanda v25.3. Previously, zero and negative values were treated the same way. Now they have distinct behaviors:
Affected Properties
segment.msretention.bytesretention.msretention.local.target.bytesretention.local.target.msinitial.retention.local.target.bytesinitial.retention.local.target.msdelete.retention.msmin.cleanable.dirty.ratioChanges Made
property-overrides.jsonwith tristate behavior detailsrpk topic createdocumentationPage previews
Checks