Skip to content

VReplication: Estimate lag when workflow fully throttled (#16577) #178

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

Merged
merged 1 commit into from
Aug 8, 2025

Conversation

mhamza15
Copy link

@mhamza15 mhamza15 commented Aug 7, 2025

This is a backport for vitessio#16577

Description

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@mhamza15 mhamza15 self-assigned this Aug 7, 2025
@Copilot Copilot AI review requested due to automatic review settings August 7, 2025 21:01
Copy link

@Copilot 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 implements lag estimation for VReplication workflows when they are fully throttled, ensuring that lag metrics remain accurate even during throttling periods.

  • Removes the timer-based heartbeat injection during throttling in vstreamer
  • Adds lag estimation functionality when throttled heartbeat events are encountered
  • Updates test helper functions to validate throttling behavior and lag calculation

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
go/vt/vttablet/tabletserver/vstreamer/vstreamer.go Simplifies throttling logic by removing timer-based heartbeats and improving logging
go/vt/vttablet/tabletmanager/vreplication/vplayer.go Adds lag estimation function and filters out throttled heartbeats from lag calculations
go/test/endtoend/vreplication/vreplication_test.go Refactors test helpers into reusable function for validating VReplication throttling metrics

estimateLag := func() {
behind := time.Now().UnixNano() - vp.lastTimestampNs - vp.timeOffsetNs
vp.vr.stats.ReplicationLagSeconds.Store(behind / 1e9)
vp.vr.stats.VReplicationLags.Add(strconv.Itoa(int(vp.vr.id)), time.Duration(behind/1e9)*time.Second)
Copy link
Preview

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

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

The lag calculation behind / 1e9 is repeated twice. Consider storing this value in a variable to avoid duplication and improve readability.

Suggested change
vp.vr.stats.VReplicationLags.Add(strconv.Itoa(int(vp.vr.id)), time.Duration(behind/1e9)*time.Second)
lagSecs := behind / 1e9
vp.vr.stats.ReplicationLagSeconds.Store(lagSecs)
vp.vr.stats.VReplicationLags.Add(strconv.Itoa(int(vp.vr.id)), time.Duration(lagSecs)*time.Second)

Copilot uses AI. Check for mistakes.

// If the batch consists only of throttled heartbeat events then we cannot
// determine the actual lag, as the vstreamer is fully throttled, and we
// will estimate it after processing the batch.
if !(event.Type == binlogdatapb.VEventType_HEARTBEAT && event.Throttled) {
Copy link
Preview

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider extracting this condition into a named function like isThrottledHeartbeat(event) to improve readability and make the logic more explicit.

Suggested change
if !(event.Type == binlogdatapb.VEventType_HEARTBEAT && event.Throttled) {
if !isThrottledHeartbeat(event) {

Copilot uses AI. Check for mistakes.

@arthurschreiber arthurschreiber merged commit ecaf20e into release-20.0-github Aug 8, 2025
179 of 187 checks passed
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