Skip to content

fix(DOC-1959): correct election_timeout_ms type/default; exclude raft_election_timeout_ms#1611

Merged
Feediver1 merged 12 commits into
mainfrom
fix/doc-1959-election-timeout-ms
May 22, 2026
Merged

fix(DOC-1959): correct election_timeout_ms type/default; exclude raft_election_timeout_ms#1611
Feediver1 merged 12 commits into
mainfrom
fix/doc-1959-election-timeout-ms

Conversation

@mfernest
Copy link
Copy Markdown
Contributor

@mfernest mfernest commented Mar 17, 2026

Summary

Fixes two issues with the election_timeout_ms cluster property reference (escalated from Support, P2):

  • election_timeout_ms was showing type string and default null — fixed via property-overrides.json to correctly show type integer and default 1500 (1500 milliseconds)
  • raft_election_timeout_ms was appearing as a separate cluster property — this is the internal C++ variable name, not the user-facing config key; marked exclude_from_docs so it no longer appears in the reference

The correct user-facing property is election_timeout_ms with a default of 1500 milliseconds.

Preview

raft_election_timeout_ms

election_timeout_ms

Test plan

  • Build passes locally
  • Verify election_timeout_ms shows type integer and default 1500 at /reference/properties/cluster-properties#election_timeout_ms
  • Verify raft_election_timeout_ms no longer appears on the cluster properties page

🤖 Generated with Claude Code

mfernest and others added 4 commits March 16, 2026 17:28
Updates property partials and JSON for v25.3.10:
- Adds "Introduced in v25.3.10" note to schema_registry_avro_use_named_references
- Adds 34 new cloud_topics topic properties
- Updates property-overrides.json with new property stubs
- Cleans up old diff JSON files

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds documentation for the new `delete_topic_enable` cluster property
(available in dev, targeting next major release):

- Adds property override with description and related_topics
- Adds Topic Deletion Control entry to the enterprise features table
  in the licensing overview

The property defaults to `true` (backward compatible). Setting it to
`false` prevents all topic deletion via the Kafka DeleteTopics API,
including for superusers, and requires an enterprise license.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…overview

- Change "Redpanda enterprise features" heading to "Redpanda Enterprise
  Edition features" for consistency with product naming on the page
- Fix Leader Pinning xref pointing to wrong anchor (Kerberos auth);
  now correctly links to develop:produce-data/leader-pinning.adoc

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_election_timeout_ms

The property docs were showing two issues:

1. election_timeout_ms had type 'string' and default 'null' — fixed via
   property-overrides.json to show correct type 'integer' and default 1500
2. raft_election_timeout_ms appeared as a separate cluster property — this
   is the C++ variable name, not the user-facing config key; marked
   exclude_from_docs so it no longer appears in the reference

The correct user-facing property is election_timeout_ms with a default
of 1500 milliseconds.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mfernest mfernest requested a review from a team as a code owner March 17, 2026 01:14
@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 17, 2026

Deploy Preview for redpanda-docs-preview ready!

Name Link
🔨 Latest commit f684cd2
🔍 Latest deploy log https://app.netlify.com/projects/redpanda-docs-preview/deploys/6a108c08de638d00079309bc
😎 Deploy Preview https://deploy-preview-1611--redpanda-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

This pull request updates Redpanda's configuration property documentation across multiple files. It adds approximately 60+ new property entries (covering cloud storage, cloud topics, coprocessor, Raft, Kafka quota, transaction, and authentication subsystems) to property override and reference metadata files, with version tagging (v25.3.10 or dev). Two version-specific property-change snapshot files are deleted. Licensing documentation is expanded with guidance on trial license extension and a new Topic Deletion Control (delete_topic_enable) feature. Configuration property documentation in cluster and topic property files is reorganized and extended, including a type change for raft_election_timeout_ms from string to integer and numerous new topic-level and cluster-level property definitions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • mattschumpert
  • micheleRP
  • Feediver1
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: fixing election_timeout_ms type/default and excluding raft_election_timeout_ms from documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description clearly explains the fixes (election_timeout_ms type/default correction and raft_election_timeout_ms exclusion) with preview links and a concrete test plan.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/doc-1959-election-timeout-ms

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
modules/reference/partials/properties/topic-properties.adoc (1)

1122-1154: ⚠️ Potential issue | 🔴 Critical

The type for delete_topic_enable should be boolean, not string.

The property description states "When set to false, all topic deletion requests are rejected," which uses boolean semantics. Enable/disable flags should be typed as boolean rather than string to accurately represent their purpose and usage.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules/reference/partials/properties/topic-properties.adoc` around lines
1122 - 1154, Update the documentation for the configuration property named
delete_topic_enable: change its Type from `string` to `boolean`, ensure the
default/null/nullable lines remain consistent with a boolean flag (e.g., `null`
or `false` as appropriate), and update any surrounding prose that assumes string
semantics (for example the line "When set to `false`, all topic deletion
requests are rejected") so it clearly references boolean values; locate the
delete_topic_enable section and adjust the Type field and any examples or
related references (e.g., restore behavior notes) to reflect a boolean typed
flag.
🧹 Nitpick comments (3)
modules/get-started/pages/licensing/overview.adoc (1)

139-139: Consider using empty brackets for the xref.

The xref could use empty brackets to automatically pull the title from the target document, which is more maintainable.

♻️ Proposed refactor
-| xref:develop:produce-data/leader-pinning.adoc[Leader Pinning]
+| xref:develop:produce-data/leader-pinning.adoc[]

Based on learnings: "AsciiDoc linking: prefer using xref links with empty brackets (e.g., xref:section/target.adoc[]) because the title is pulled from the referenced document automatically."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules/get-started/pages/licensing/overview.adoc` at line 139, Update the
xref to use empty brackets so the target document's title is pulled
automatically: replace the current xref reference
`xref:develop:produce-data/leader-pinning.adoc[Leader Pinning]` with
`xref:develop:produce-data/leader-pinning.adoc[]` in the licensing overview page
to improve maintainability.
modules/reference/attachments/redpanda-properties-v25.3.10.json (2)

5486-5505: Consider using "type": "boolean" for delete_topic_enable.

The description states "When set to false, all topic deletion requests are rejected...", indicating this is a boolean property. However, "type": "string" is specified. If the override system requires string types, this is fine; otherwise, consider changing to "type": "boolean" for accuracy.

Suggested change if boolean type is supported
         "name": "delete_topic_enable",
         "related_topics": [
             "xref:reference:properties/cluster-properties.adoc#kafka_nodelete_topics[`kafka_nodelete_topics`]",
             "xref:get-started:licensing/index.adoc[enterprise license]"
         ],
-        "type": "string",
+        "type": "boolean",
         "version": "dev",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules/reference/attachments/redpanda-properties-v25.3.10.json` around lines
5486 - 5505, The schema for the property delete_topic_enable is declared with
"type": "string" but the description and semantics indicate a boolean; update
the property definition for delete_topic_enable to use "type": "boolean" (or the
equivalent boolean/schema representation your override system expects), ensuring
any default/null handling and validation logic for the override system are
adjusted accordingly (e.g., change "default": null if booleans require
true/false defaults or adapt parsing code that reads delete_topic_enable).

2688-2703: Placeholder descriptions need enrichment before GA release.

Multiple new properties (e.g., cloud_storage_prefetch_segments_max, cloud_topics_compaction_interval_ms, etc.) have placeholder descriptions like "Configuration property: {name}". Since these are marked as "version": "dev", please ensure meaningful descriptions are added before these properties reach GA to provide useful documentation for users.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules/reference/attachments/redpanda-properties-v25.3.10.json` around lines
2688 - 2703, Several properties (e.g., cloud_storage_prefetch_segments_max,
cloud_topics_compaction_interval_ms) currently have placeholder descriptions
like "Configuration property: {name}"; update each property entry (look for name
fields such as "cloud_storage_prefetch_segments_max" and
"cloud_topics_compaction_interval_ms") to replace the placeholder description
with a concise, meaningful explanation of the setting, its accepted values/type,
default/behavior, and any scope or impact (topic vs cluster) so the JSON doc is
informative before GA; ensure description text matches the "type",
"is_topic_property", and "visibility" metadata for each property.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@modules/reference/attachments/redpanda-properties-v25.3.10.json`:
- Around line 10310-10325: The three properties currently marked with
"config_scope": "topic" need to be changed to cluster-level: update
"oidc_group_claim_path",
"partition_autobalancing_node_autodecommission_timeout_sec", and
"internal_rpc_request_timeout_ms" so their "config_scope" value is "cluster"
(matching the other OIDC and partition_autobalancing entries and RPC timeout
properties); keep their other fields (name, version, visibility, etc.) unchanged
and ensure the change is applied to the JSON entries for the properties named
oidc_group_claim_path,
partition_autobalancing_node_autodecommission_timeout_sec, and
internal_rpc_request_timeout_ms.

---

Outside diff comments:
In `@modules/reference/partials/properties/topic-properties.adoc`:
- Around line 1122-1154: Update the documentation for the configuration property
named delete_topic_enable: change its Type from `string` to `boolean`, ensure
the default/null/nullable lines remain consistent with a boolean flag (e.g.,
`null` or `false` as appropriate), and update any surrounding prose that assumes
string semantics (for example the line "When set to `false`, all topic deletion
requests are rejected") so it clearly references boolean values; locate the
delete_topic_enable section and adjust the Type field and any examples or
related references (e.g., restore behavior notes) to reflect a boolean typed
flag.

---

Nitpick comments:
In `@modules/get-started/pages/licensing/overview.adoc`:
- Line 139: Update the xref to use empty brackets so the target document's title
is pulled automatically: replace the current xref reference
`xref:develop:produce-data/leader-pinning.adoc[Leader Pinning]` with
`xref:develop:produce-data/leader-pinning.adoc[]` in the licensing overview page
to improve maintainability.

In `@modules/reference/attachments/redpanda-properties-v25.3.10.json`:
- Around line 5486-5505: The schema for the property delete_topic_enable is
declared with "type": "string" but the description and semantics indicate a
boolean; update the property definition for delete_topic_enable to use "type":
"boolean" (or the equivalent boolean/schema representation your override system
expects), ensuring any default/null handling and validation logic for the
override system are adjusted accordingly (e.g., change "default": null if
booleans require true/false defaults or adapt parsing code that reads
delete_topic_enable).
- Around line 2688-2703: Several properties (e.g.,
cloud_storage_prefetch_segments_max, cloud_topics_compaction_interval_ms)
currently have placeholder descriptions like "Configuration property: {name}";
update each property entry (look for name fields such as
"cloud_storage_prefetch_segments_max" and "cloud_topics_compaction_interval_ms")
to replace the placeholder description with a concise, meaningful explanation of
the setting, its accepted values/type, default/behavior, and any scope or impact
(topic vs cluster) so the JSON doc is informative before GA; ensure description
text matches the "type", "is_topic_property", and "visibility" metadata for each
property.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d361af3a-4321-4381-9790-0efefba3e055

📥 Commits

Reviewing files that changed from the base of the PR and between fe8a357 and e8de67a.

📒 Files selected for processing (7)
  • docs-data/property-overrides.json
  • docs-data/redpanda-property-changes-v25.3.1-to-v25.3.3.json
  • docs-data/redpanda-property-changes-v25.3.9-to-v25.3.10.json
  • modules/get-started/pages/licensing/overview.adoc
  • modules/reference/attachments/redpanda-properties-v25.3.10.json
  • modules/reference/partials/properties/cluster-properties.adoc
  • modules/reference/partials/properties/topic-properties.adoc
💤 Files with no reviewable changes (2)
  • docs-data/redpanda-property-changes-v25.3.1-to-v25.3.3.json
  • docs-data/redpanda-property-changes-v25.3.9-to-v25.3.10.json

Comment on lines +10310 to +10325
"oidc_group_claim_path": {
"cloud_byoc_only": false,
"cloud_editable": false,
"cloud_readonly": false,
"cloud_supported": false,
"config_scope": "topic",
"default": null,
"defined_in": "override",
"description": "Configuration property: oidc_group_claim_path",
"is_deprecated": false,
"is_topic_property": true,
"name": "oidc_group_claim_path",
"type": "string",
"version": "dev",
"visibility": "user"
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are other similar OIDC properties and their config_scope
rg -n '"oidc_' modules/reference/attachments/redpanda-properties-v25.3.10.json | head -20

# Check the config_scope of other partition_autobalancing properties for consistency
rg -n '"partition_autobalancing_' modules/reference/attachments/redpanda-properties-v25.3.10.json | head -30

Repository: redpanda-data/docs

Length of output: 2001


🏁 Script executed:

#!/bin/bash
# Extract full OIDC properties with their config_scope
rg -A 13 '"oidc_clock_skew_tolerance": {' modules/reference/attachments/redpanda-properties-v25.3.10.json | grep -E '(oidc_|config_scope|is_topic_property)' | head -30
echo "---"
# Extract sample partition_autobalancing properties with their config_scope
rg -A 13 '"partition_autobalancing_concurrent_moves": {' modules/reference/attachments/redpanda-properties-v25.3.10.json | grep -E '(partition_autobalancing|config_scope|is_topic_property)' | head -20

Repository: redpanda-data/docs

Length of output: 360


🏁 Script executed:

#!/bin/bash
# Extract OIDC properties around line 10272-10390 to see config_scope values
sed -n '10272,10390p' modules/reference/attachments/redpanda-properties-v25.3.10.json | grep -E '"config_scope"|"is_topic_property"|"name"|oidc_'

echo "=== partition_autobalancing properties ==="
# Extract partition properties around line 10472-10530 to see config_scope values
sed -n '10472,10530p' modules/reference/attachments/redpanda-properties-v25.3.10.json | grep -E '"config_scope"|"is_topic_property"|"name"|partition_autobalancing_'

Repository: redpanda-data/docs

Length of output: 1730


🏁 Script executed:

#!/bin/bash
# Check nested_group_behavior
sed -n '10130,10145p' modules/reference/attachments/redpanda-properties-v25.3.10.json | grep -E '"config_scope"|"is_topic_property"|"name"'

echo "==="
# Check internal_rpc_request_timeout_ms
sed -n '7574,7589p' modules/reference/attachments/redpanda-properties-v25.3.10.json | grep -E '"config_scope"|"is_topic_property"|"name"'

echo "==="
# Check partition_autobalancing_node_autodecommission_timeout_sec
sed -n '10588,10603p' modules/reference/attachments/redpanda-properties-v25.3.10.json | grep -E '"config_scope"|"is_topic_property"|"name"'

Repository: redpanda-data/docs

Length of output: 478


Fix config_scope for properties incorrectly marked as topic-level.

The following properties are marked as "config_scope": "topic" but should be cluster-level:

  • oidc_group_claim_path: All other OIDC properties (oidc_clock_skew_tolerance, oidc_discovery_url, oidc_keys_refresh_interval, oidc_principal_mapping, oidc_token_audience) are correctly set to "config_scope": "cluster". This property should match.
  • partition_autobalancing_node_autodecommission_timeout_sec: All other partition_autobalancing_* properties are correctly set to "config_scope": "cluster". This property should match.
  • internal_rpc_request_timeout_ms: RPC timeout configuration is cluster-level, not topic-level.

These properties are marked as "dev" version but have inconsistent or incorrect config_scope values that should be corrected before these properties go GA.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules/reference/attachments/redpanda-properties-v25.3.10.json` around lines
10310 - 10325, The three properties currently marked with "config_scope":
"topic" need to be changed to cluster-level: update "oidc_group_claim_path",
"partition_autobalancing_node_autodecommission_timeout_sec", and
"internal_rpc_request_timeout_ms" so their "config_scope" value is "cluster"
(matching the other OIDC and partition_autobalancing entries and RPC timeout
properties); keep their other fields (name, version, visibility, etc.) unchanged
and ensure the change is applied to the JSON entries for the properties named
oidc_group_claim_path,
partition_autobalancing_node_autodecommission_timeout_sec, and
internal_rpc_request_timeout_ms.

…ions section

Rewrites the opening of the "Partition leadership elections" section in
architecture.adoc:

- Clarifies the distinction between heartbeat interval (150ms) and
  election timeout (1500ms)
- Removes the term definition (was defined after its first use)
- Adds inline definition of "term" on first use
- Links election_timeout_ms cluster property with default value

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mfernest
Copy link
Copy Markdown
Contributor Author

@coderabbitai Same note as PR #1610: delete_topic_enable does not appear in topic-properties.adoc on this branch. Line 1122 in that file is delete.retention.ms. The delete_topic_enable property is correctly placed in cluster-properties.adoc only, typed as boolean with default true. This comment appears to be a false positive.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 17, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if delete_topic_enable appears in topic-properties.adoc
echo "=== Searching for delete_topic_enable in topic-properties.adoc ==="
rg -n "delete_topic_enable" modules/reference/partials/properties/topic-properties.adoc || echo "NOT FOUND in topic-properties.adoc"

# Check line 1122 of topic-properties.adoc
echo "=== Lines 1120-1125 of topic-properties.adoc ==="
sed -n '1120,1125p' modules/reference/partials/properties/topic-properties.adoc

# Confirm delete_topic_enable in cluster-properties.adoc
echo "=== delete_topic_enable occurrences in cluster-properties.adoc ==="
rg -n "delete_topic_enable" modules/reference/partials/properties/cluster-properties.adoc | head -20

Length of output: 361


@mfernest: After checking the branch directly, delete_topic_enable does appear in modules/reference/partials/properties/topic-properties.adoc — the script confirms it at Line 1122:

=== delete_topic_enable

And again at Line 1126:

Configuration property: delete_topic_enable

It is also present in cluster-properties.adoc at Line 4137.

So the review comment is not a false positive — the property currently exists in both files on this branch. If delete_topic_enable is intended to be a cluster-only property, its entry in topic-properties.adoc (introduced with a dev tag) would need to be removed.

@mfernest mfernest requested a review from dotnwat March 18, 2026 17:10
@mfernest
Copy link
Copy Markdown
Contributor Author

@dotnwat — gentle nudge on this one when you have a chance. Corrects the election_timeout_ms type and default value for DOC-1959/DOC-1960.

Copy link
Copy Markdown
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@micheleRP micheleRP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

What this PR actually fixes (DOC-1959)

The core fix is valuable: election_timeout_ms was showing the wrong type (string instead of integer) and wrong default (null instead of 1500). This was escalated from Support as P2. The fix is in property-overrides.json:

"election_timeout_ms": {
  "type": "integer",
  "default": 1500,
  ...
}

And correctly excludes the internal C++ variable raft_election_timeout_ms from docs:

"raft_election_timeout_ms": {
  "exclude_from_docs": true,
  "config_scope": "cluster"
}

Both of these are in property-overrides.json — the correct mechanism. They will survive automation regeneration.

Property automation impact

Same pattern as PRs #1607 and #1610. Files that should be removed:

File Reason
modules/reference/partials/properties/cluster-properties.adoc (+238) Auto-generated, will be overwritten
modules/reference/partials/properties/topic-properties.adoc (+1095/-43) Auto-generated, will be overwritten
modules/reference/attachments/redpanda-properties-v25.3.10.json (+574/-41) Auto-generated, will be overwritten
docs-data/redpanda-property-changes-v25.3.1-to-v25.3.3.json (deleted) Generated artifact
docs-data/redpanda-property-changes-v25.3.9-to-v25.3.10.json (deleted) Generated artifact

These generated files contain 31 instances of *Introduced in dev* that would appear in published docs.

Files that should stay

File Content
docs-data/property-overrides.json The election_timeout_ms type/default fix + raft_election_timeout_ms exclusion — the core fix
modules/get-started/pages/architecture.adoc Improved Raft election description with xref to election_timeout_ms
modules/get-started/pages/licensing/overview.adoc Heading fix + Leader Pinning xref fix

Major overlap with PRs #1607 and #1610

This PR shares 7 files with PR #1610 and 4 files with PR #1607. The licensing/overview.adoc changes are identical to #1610 (same heading fix, same Leader Pinning xref fix). The property-overrides.json changes share the same bulk automation additions (v25.3.10 version tags, dev properties) across all three PRs.

It looks like all three PRs ran the property automation independently and each bundled the shared generated output alongside their unique manual changes. If the generated files are removed from all three PRs, the conflicts collapse to just property-overrides.json (which would merge cleanly since each PR adds different property entries) and licensing/overview.adoc (duplicated between #1610 and #1611 — one should own these changes).

Style review of manual content

architecture.adoc change — good improvement:

  • Replaced vague "over a period of time (default = 1.5 seconds)" with an explicit xref to election_timeout_ms (default: 1500 milliseconds)
  • Changed "A term is an arbitrary period of time that starts when a leader election is triggered" to the parenthetical "(an election cycle)" — more concise
  • Changed "periodic heartbeat" to "heartbeat every 150 milliseconds" — more specific

licensing/overview.adoc — identical to PR #1610. Only one PR should include these changes.

Recommendation

  1. Remove the 5 generated files — they add ~1900 lines of unreviewable content with "Introduced in dev" artifacts
  2. Remove licensing/overview.adoc — it's already in PR #1610. Having it in both will cause merge conflicts
  3. Keep property-overrides.json (the election_timeout_ms fix + raft exclusion) and architecture.adoc (the Raft description improvement)
  4. Strip the bulk automation additions from property-overrides.json (the v25.3.10 version tags, dev properties) — these are shared across all three PRs and should be handled in a single dedicated automation PR

Copy link
Copy Markdown
Contributor

@micheleRP micheleRP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mfernest please see Claude's review

mfernest and others added 2 commits March 24, 2026 16:52
Remove property partials and JSON attachments included by the auto-docs
regen commit. Only property-overrides.json and content pages belong here.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mfernest mfernest requested a review from micheleRP March 24, 2026 23:55
@micheleRP
Copy link
Copy Markdown
Contributor

Review: Claude feedback incorporation check

Checking whether the 4 recommendations from the Claude (micheleRP) review have been addressed.

1. Remove the 5 generated files — ✅ Done

Commit 22e0a63 stripped all auto-generated property files. The PR now only touches 3 files.

2. Remove licensing/overview.adoc (duplicate of PR #1610) — ❌ Not done

The licensing/overview.adoc changes are still in this PR. The heading fix (Redpanda enterprise featuresRedpanda Enterprise Edition features) and Leader Pinning xref fix are identical to PR #1610. Having them in both PRs will cause merge conflicts. The new delete_topic_enable row in the licensing table may be intentional for this PR, but the heading and xref fixes should live in only one PR.

3. Keep property-overrides.json and architecture.adoc — ✅ Done

Both files are retained.

4. Strip bulk automation version tags from property-overrides.json — ❌ Not done

The diff still contains ~60+ bulk version-tag entries (e.g. "alter_topic_cfg_timeout_ms": { "version": "v25.3.10" }, "cloud_topics_compaction_interval_ms": { "version": "dev" }, etc.) that are shared across PRs #1607, #1610, and #1611. The review recommended these go in a single dedicated automation PR.

The core fix unique to this PR is just:

  • election_timeout_ms: "type": "integer", "default": 1500
  • raft_election_timeout_ms: "exclude_from_docs": true
  • delete_topic_enable: new property entry with description and related topics

Summary

Recommendation Status
Remove 5 generated files ✅ Done
Remove licensing/overview.adoc (duplicate of #1610) ❌ Not done
Keep property-overrides.json + architecture.adoc ✅ Done
Strip bulk automation version tags from property-overrides.json ❌ Not done

Note: The CodeRabbit feedback about config_scope issues and delete_topic_enable type is no longer relevant — those findings were in the generated files that have since been removed.

mfernest and others added 4 commits March 26, 2026 09:01
…s.json

Strip bulk automation entries that only carried version tags. These belong
in a dedicated automation PR. Retain the intentional changes: updated
election_timeout_ms override (corrected type to integer, default 1500) and
new raft_election_timeout_ms entry (exclude_from_docs).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@Feediver1 Feediver1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: fix(DOC-1959): correct election_timeout_ms type/default; exclude raft_election_timeout_ms (#1611)

Files reviewed: 2 .adoc/.json files (7 additions / 1 deletion)
Overall assessment: Clean and minimal post-cleanup. DOC-1959 is satisfied exactly. All three of @micheleRP's March 24 review concerns were addressed by follow-up commits on March 24–26. No new findings on the current state. The CHANGES_REQUESTED block on this PR is stale.

What this PR does

Two targeted fixes against the Support-escalated bug in DOC-1959, plus a small bonus prose improvement:

  • docs-data/property-overrides.json — adds type: integer and default: 1500 to the election_timeout_ms entry (was rendering as string / null); marks raft_election_timeout_ms as exclude_from_docs: true (it's the internal C++ variable name, not a user-facing config key).
  • modules/get-started/pages/architecture.adoc — rewrites the Raft heartbeat / election-timeout sentence in the "Partition leadership elections" section to use concrete timings inline ("every 150 milliseconds", "default: 1500 milliseconds") and inserts an xref to the now-correct election_timeout_ms property so readers can configure it.

Jira ticket alignment

Ticket: DOC-1959 — Support P2: election_timeout_ms was showing wrong type (string instead of integer) and wrong default (null instead of 1500). raft_election_timeout_ms was appearing as a separate cluster property but is actually the internal C++ variable name.

Status:Satisfies the ticket exactly. All three corrections (type, default, raft_-prefix exclusion) are in the property-overrides.json diff, which is the correct mechanism — these will survive future property-docs regenerations.

Critical issues (must fix)

None.

Suggestions (should consider)

None of substance. Ran the prose-cleanup-pr-review checks on the architecture.adoc change:

  • Information-loss audit: the previous text defined "term" as "an arbitrary period of time that starts when a leader election is triggered". The rewrite compresses this to the parenthetical "(an election cycle)". Concise but adequate — the surrounding paragraph uses "term" in context three more times, and the new sentence about election_timeout_ms makes the election-cycle framing concrete.
  • Warning-tone scrutiny: no warning/note in the change. N/A.

Impact on other files

  • The new xref resolves. xref:reference:properties/cluster-properties.adoc#election_timeout_ms[\election_timeout_ms`]— verified the=== election_timeout_msheading exists inmodules/reference/partials/properties/cluster-properties.adoc:5411`.
  • raft_election_timeout_ms removal will surface after merge. Currently still rendered at line 14939 of the partial; the exclude_from_docs: true override will suppress it on the next property-docs regeneration job.
  • No What's New entry needed — bug fix to existing reference data, not a user-visible feature.
  • No cross-repo / single-source impact — these files are self-managed-only.

Outstanding review activity — status

  • @dotnwat APPROVED on 2026-03-23 ("LGTM"). ✓

  • @micheleRP CHANGES_REQUESTED on 2026-03-24 with three structural concerns:

    Concern Resolved in commit Outcome
    Remove 5 auto-generated property files 22e0a63f (2026-03-24) ✓ stripped
    Remove licensing/overview.adoc (duplicate of PR #1610) 5c104b2e (2026-03-26) ✓ removed
    Strip bulk version=dev entries from property-overrides.json 723050ae (2026-03-26) ✓ removed

    The current diff is exactly the minimal scope Michele's review recommended. Her CHANGES_REQUESTED state hasn't been refreshed since her concerns landed, so the PR is still BLOCKED on a stale review.

    @micheleRP — would you take another look and dismiss/refresh, given the cleanup landed in commits 22e0a63f / 5c104b2e / 723050ae?

CodeRabbit findings worth considering

The one inline comment CodeRabbit posted (on modules/reference/attachments/redpanda-properties-v25.3.10.json:10142, about oidc_* and partition_autobalancing_* config_scope consistency) was on an auto-generated file that mfernest later stripped from the PR (commit 22e0a63f). The comment is effectively moot.

What works well

  • Hits the DOC-1959 spec exactly. Type, default, and the raft_election_timeout_ms exclusion all match the ticket. The fixes are in property-overrides.json — the correct mechanism — so they survive automation regeneration.
  • Bonus prose improvement is a real quality upgrade. The architecture.adoc rewrite uses concrete timings in prose, consistent units (milliseconds throughout — no more mixing "1.5 seconds" and "1500 milliseconds"), and an inline xref so readers can act on what they just learned.
  • Author was responsive to review. All three of @micheleRP's structural concerns were addressed in dedicated, well-named follow-up commits within two days of the review landing.
  • Branch is current with main (merge from main on 2026-05-22), so no rebase work pending.
  • CI fully green.

Final-pass review via /docs-team-standards:pr-review.

@micheleRP
Copy link
Copy Markdown
Contributor

Final-pass review — ready to merge

Re-reviewed the current state of this PR against main. Net diff is just 2 files (+7 / -1):

  • docs-data/property-overrides.json — adds type: integer + default: 1500 to election_timeout_ms; marks raft_election_timeout_ms with exclude_from_docs: true
  • modules/get-started/pages/architecture.adoc — rewrites the Raft heartbeat / election-timeout sentence with concrete timings and an xref to election_timeout_ms

DOC-1959 is satisfied exactly. Fixes are in property-overrides.json (the correct mechanism — they survive property-docs regen). xref target verified.

Status of my 2026-03-24 CHANGES_REQUESTED

All three concerns were addressed in follow-up commits — the review state just wasn't refreshed:

Concern Resolved in commit
Remove 5 auto-generated property files 22e0a63
Remove licensing/overview.adoc (dup of #1610) 5c104b2
Strip bulk version=dev entries from property-overrides.json 723050a

Findings on current diff

  • Critical: none
  • Suggestions: none of substance
  • CodeRabbit: the one inline comment was on a generated file that's been stripped — moot
  • Impact: no other .adoc pages reference these properties; no orphan xrefs; no What's New / nav / single-source impact

Recommendation

Dismissing my stale review and squash-merging. The author has left the company, the work is done, and the fix is still needed (main currently has the broken entry).

@Feediver1 Feediver1 merged commit 4adb548 into main May 22, 2026
7 checks passed
@Feediver1 Feediver1 deleted the fix/doc-1959-election-timeout-ms branch May 22, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants