Skip to content

[KafkaIO] Use UnsignedLong for backlog calculation - #40840

Merged
Abacn merged 1 commit into
apache:masterfrom
sjvanrossum:kafkaio-backlog-unsigned-math
Oct 28, 2026
Merged

Abacn merged 1 commit into
apache:masterfrom
sjvanrossum:kafkaio-backlog-unsigned-math

Conversation

@sjvanrossum

Copy link
Copy Markdown
Contributor

Addresses a comment to align backlog calculation with changes made to growable offset trackers.

@github-actions github-actions Bot added java io kafka size/s S: 10-49 lines changed labels Oct 27, 2026

@github-actions github-actions Bot left a comment

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.

📋 Review Summary

This PR updates ReadFromKafkaDoFn to calculate estimated backlog bytes using Guava's UnsignedLong instead of BigDecimal, aligning it with the progress calculation in GrowableOffsetRangeTracker. It also updates outdated comments and log messages regarding offset estimator caching and lifecycle management (setup/teardown).

🔍 General Feedback

  • The mathematical conversion using UnsignedLong.fromLongBits(Math.max(expectedOffset, latestOffsetEstimator.get()) - expectedOffset) safely handles potential 64-bit signed integer overflow (since the minuend is guaranteed to be greater than or equal to the subtrahend) while avoiding BigDecimal allocation overhead on the hot path.
  • Comment and log message updates accurately reflect the current caching and DoFn lifecycle behavior.

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

@github-actions

Copy link
Copy Markdown
Contributor

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

@Abacn Abacn left a comment

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.

Thanks! Nice optimization. Overflow case also covered by UnsignedLong.fromLongBits

@Abacn
Abacn merged commit 5307a31 into apache:master Oct 28, 2026
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

io java kafka size/s S: 10-49 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants