Skip to content

Conversation

@paulohtb6
Copy link
Collaborator

@paulohtb6 paulohtb6 commented Oct 28, 2025

Description

Resolves https://redpandadata.atlassian.net/browse/DOCS-1758
Review deadline: Oct 31st

Related code PR redpanda-data/redpanda#28233

Page previews

Transactions - tune producer ID limits

max_concurrent_producer_ids reference

transactional_id_expiration_ms reference

Checks

  • New feature
  • Content gap
  • Support Follow-up
  • Small fix (typos, links, copyedits, etc)

@paulohtb6 paulohtb6 requested a review from a team as a code owner October 28, 2025 14:31
@netlify
Copy link

netlify bot commented Oct 28, 2025

Deploy Preview for redpanda-docs-preview ready!

Name Link
🔨 Latest commit 0d151e8
🔍 Latest deploy log https://app.netlify.com/projects/redpanda-docs-preview/deploys/6900d3c5fec5410008faa1d6
😎 Deploy Preview https://deploy-preview-1425--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
Contributor

coderabbitai bot commented Oct 28, 2025

📝 Walkthrough

Walkthrough

This PR updates Redpanda transaction documentation with operational guidance. It adds two new best-practices sections to the transactions guide—one covering producer ID limits with configuration bounds and monitoring metrics, and another addressing transaction timeout configuration and coordinator-side tuning. Additionally, it refines the descriptions of max_concurrent_producer_ids and transactional_id_expiration_ms cluster properties with clarifications about per-shard tracking, LRU eviction, memory management concerns, and cross-references between related topics.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Changes are purely documentation additions and refinements across two .adoc files
  • Straightforward content additions with consistent formatting
  • Minor attention to ensure cross-references between transaction guidance and cluster property descriptions are accurate
  • Verify that recommended tuning values and metrics (evicted_producers, total_active_producers) align with current product behavior

Possibly related PRs

  • redpanda-data/docs#1257 — Overlaps with transaction documentation and cluster property updates (e.g., transactional_id_expiration_ms, max_concurrent_producer_ids, transaction timeout guidance)

Suggested reviewers

  • jason-da-redpanda
  • Feediver1
  • bharathv

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "transactions: add info about preventing OOM" accurately captures the main objective of the changeset. The changes span two documentation files focused on guiding users to configure transaction-related properties (max_concurrent_producer_ids and transactional_id_expiration_ms) to prevent Out-of-Memory (OOM) issues. The title is concise, specific, and clearly communicates the primary intent without unnecessary noise or vague terms.
Description Check ✅ Passed The pull request description comprehensively follows the provided template structure and includes all required sections. The Description section properly includes a JIRA ticket link in the correct format (DOCS-1758), a specified review deadline (Oct 31st), and an additional helpful reference to the related code PR. The Page previews section provides three specific Netlify preview URLs with the correct format and relevant sections. The Checks section includes all available options with "Small fix (typos, links, copyedits, etc)" appropriately selected. The content is complete, specific, and not vague or generic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-prop

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • DOCS-1758: Request failed with status code 404

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
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: 0

Caution

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

⚠️ Outside diff range comments (2)
modules/develop/pages/transactions.adoc (2)

303-305: Add explicit anchor for transaction coordinator disk management section.

The section "Manage transaction coordinator disk usage" exists at line 166 in disk-utilization.adoc, but the anchor [[manage-transaction-coordinator-disk-usage]] is not explicitly defined. Add the anchor before the section heading to fix the broken reference:

[[manage-transaction-coordinator-disk-usage]]
== Manage transaction coordinator disk usage

298-302: Update line 298 xref to match the consistent format used elsewhere in the file.

The reference to max_transactions_per_coordinator on line 298 uses an incomplete path format. Other property references in the same file (lines 275, 290) use the format xref:reference:properties/cluster-properties.adoc#, but line 298 uses xref:reference:cluster-properties#, which is missing the /properties/ path component and .adoc extension. Update it to:

xref:reference:properties/cluster-properties.adoc#max_transactions_per_coordinator
🧹 Nitpick comments (1)
modules/develop/pages/transactions.adoc (1)

279-281: Clarify the upper bound formula definition.

The upper bound formula (topic_partitions_per_shard × kafka_connections_max) may be ambiguous. The reasoning states it "assumes producers connect to all shards," but the mathematical interpretation could be unclear to users:

  • Does this mean: (partitions per shard) × (total connections in cluster)?
  • Or: For each of the (partitions per shard), multiply by the connections that might touch that partition?

Consider rephrasing to be more explicit about what this maximum represents, or provide a concrete example to illustrate the calculation.

📜 Review details

Configuration used: CodeRabbit 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a829416 and 0d151e8.

📒 Files selected for processing (2)
  • modules/develop/pages/transactions.adoc (1 hunks)
  • modules/reference/pages/properties/cluster-properties.adoc (4 hunks)
⏰ 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 (4)
modules/reference/pages/properties/cluster-properties.adoc (2)

6357-6379: Cross-referenced sections match between properties.

The Related topics section correctly references the same tune-producer-id-limits anchor and provides bidirectional cross-linking with max_concurrent_producer_ids. This creates good navigation between related configuration concerns.


4067-4089: No issues found—cross-reference is valid and correctly anchored.

The section "=== Tune producer ID limits" exists at line 273 in modules/develop/pages/transactions.adoc with the correct auto-generated anchor tune-producer-id-limits. The cross-reference in the cluster properties file will resolve correctly.

modules/develop/pages/transactions.adoc (2)

287-289: No issues found. Both referenced metrics exist in internal-metrics-reference.adoc with matching anchor definitions. The xref links will resolve correctly.


273-307: No action required—anchor reference is correctly structured.

The section === Tune producer ID limits at line 273 in transactions.adoc will automatically generate the anchor tune-producer-id-limits, which matches the cross-reference from cluster-properties.adoc at lines 4071, 4085, and 6377. The xref syntax is correct and will resolve properly without requiring an explicit anchor directive.

@jason-da-redpanda
Copy link
Contributor

jason-da-redpanda commented Oct 29, 2025

Not sure how many self-hosted customers set kafka_connections_max

Even if they do ...there can be cases where clients do not efficiently use producer-ids .. generating more producer-ids than there are connections.../never re-used... (such as flink in wiki example) ..
The ticket that generated this issue,... in the bundle we saw > 4 million connections..
yet a small amount of connections. (few hundred)

So it might be worth having some notes on best practices regarding client apps and producer-ids
Ideally, customers need to understand what their applications are up to in terms of producer-ids..
We mention the metrics which help monitor these.. so that is great (support will create follow up to make these metrics in the public metrics.. )

Additionally.. transactional_id_expiration_ms is worth mentioning . the default is 7 days... which is way more than most transactions are typically open for.. and setting that to a sane value can help in this area too..

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.

3 participants