-
Notifications
You must be signed in to change notification settings - Fork 34
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
add Tombstone property #847
Conversation
✅ Deploy Preview for redpanda-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Apply suggestions from code review remove iceberg
2f4eec9
to
820e238
Compare
|
||
*Accepted values:* [`-17592186044416`, `17592186044415`] | ||
|
||
*Default:* `null` |
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.
Not sure if we usually add recommended values for properties that default to null
in the docs, but up to your discretion as to whether we want to mention a typical default of 24 hours
/86400000 ms
(see kafka docs here).
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.
If we have recommended values we can add them in the description cc @kbatuigas
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.
@WillemKauf per the code, look like this accepts negative values. Should this be a bounded_property to avoid this? I can add 0 as starting point too.
Asking because someties allowing negative numbers in time-based properties is intended
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 valid range is actually defined here: [1ms, 17592186044415]
is correct.
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 notice some other time-based properties that might need fixing:
https://docs.redpanda.com/current/reference/properties/cluster-properties/#raft_replica_max_flush_delay_ms is also bounded by [1ms, 17592186044415]
but is negative in the docs.
https://docs.redpanda.com/current/reference/properties/cluster-properties/#raft_heartbeat_interval_ms is also bounded by [1ms, 17592186044415]
but is negative in the docs.
There are a lot more properties in core that should have these bounds enforced, but don't... hm.
|
||
*Type:* integer | ||
|
||
*Accepted values:* [`-17592186044416`, `17592186044415`] |
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.
[1ms, 17592186044415]
@@ -4856,6 +4856,24 @@ Transaction manager's synchronization timeout. Maximum time to wait for internal | |||
|
|||
--- | |||
|
|||
=== tombstone_retention_ms | |||
|
|||
The retention time for tombstone records in a compacted topic. Cannot be enabled at the same time as any of `cloud_storage_enabled`, `cloud_storage_enable_remote_read`, or `cloud_storage_enable_remote_write`. |
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 retention time for tombstone records in a compacted topic. Cannot be enabled at the same time as any of `cloud_storage_enabled`, `cloud_storage_enable_remote_read`, or `cloud_storage_enable_remote_write`. | |
The retention time (in ms) for tombstone records in a compacted topic. Cannot be enabled at the same time as any of `cloud_storage_enabled`, `cloud_storage_enable_remote_read`, or `cloud_storage_enable_remote_write`. A typical default setting is `86400000`, or 24 hours. |
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.
Not sure if we should add (in ms) as we already have the units field
*Accepted values:* [`-17592186044416`, `17592186044415`] | ||
|
||
*Default:* `null` | ||
|
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.
We could also add the main doc as a related topic? The xref will be xref:manage:cluster-maintenance/compaction-settings.adoc#tombstone-record-removal[Tombstone record removal]
Changelog
Added properties
tombstone_retention_ms
Description
Resolves https://github.com/redpanda-data/documentation-private/issues/
Review deadline:
Page previews
Checks