Skip to content

Don't return negative offset lags except for -1 when -1001 is the high watermark#406

Merged
Emanuele Sabellico (emasab) merged 2 commits intomasterfrom
dev_avoid_negative_offset_lags
Dec 1, 2025
Merged

Don't return negative offset lags except for -1 when -1001 is the high watermark#406
Emanuele Sabellico (emasab) merged 2 commits intomasterfrom
dev_avoid_negative_offset_lags

Conversation

@emasab
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings December 1, 2025 13:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modifies the consumer lag calculation logic to handle edge cases with watermark offsets. The changes prevent returning negative offset lags by introducing validation for the high watermark offset and clamping calculated lag values to zero when they would otherwise be negative.

Key Changes:

  • Added validation to skip lag calculation when high watermark is the invalid sentinel value (-1001)
  • Introduced clamping logic to ensure offset lag values are never negative

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +932 to +933
offsetLag_ = offsetLag_ > 0 ? offsetLag_ : 0;
offsetLagLow_ = offsetLagLow_ > 0 ? offsetLagLow_ : 0;
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

The ternary condition uses > 0 which excludes zero lag as a valid value. This should use >= 0 to preserve zero lag, or the logic should clamp negative values: offsetLag_ = Math.max(0, offsetLag_) to be clearer about the intent.

Suggested change
offsetLag_ = offsetLag_ > 0 ? offsetLag_ : 0;
offsetLagLow_ = offsetLagLow_ > 0 ? offsetLagLow_ : 0;
offsetLag_ = Math.max(0, offsetLag_);
offsetLagLow_ = Math.max(0, offsetLagLow_);

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

considered that but don't want to involve the additional call for this simple logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM!

@emasab Emanuele Sabellico (emasab) merged commit 8241d96 into master Dec 1, 2025
2 checks passed
@emasab Emanuele Sabellico (emasab) deleted the dev_avoid_negative_offset_lags branch December 1, 2025 17:01
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