Skip to content

ref(metrics): Rework how replication lag is computed - #1114

Merged
iambriccardo merged 4 commits into
mainfrom
fix-lsn-check
Oct 17, 2026
Merged

iambriccardo merged 4 commits into
mainfrom
fix-lsn-check

Conversation

@iambriccardo

@iambriccardo iambriccardo commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

This PR improves how replication lag is computed by using a new single task that can consistently emit lag metrics of multiple workers.

@iambriccardo
iambriccardo marked this pull request as ready for review October 16, 2026 09:54
@iambriccardo
iambriccardo requested a review from a team as a code owner October 16, 2026 09:54

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I reviewed this PR and didn't find any bugs. It's a large redesign of how lag is measured: it replaces five existing gauges with two new ones and adds a pipeline-owned sampler task, so a human look from the pipelines owners would still be worthwhile.

What was reviewed:

  • The new ReplicationLagMonitor and ReplicationLagMetrics in replication/lag.rs: weak worker registrations, taking the progress snapshot before the source query, the max-per-path aggregation, the -1 sentinel, and the reset-on-drop in MetricsLifetime.
  • How the sampler is spawned and torn down through AbortOnDropHandle in pipeline.rs, plus the removal of the per-apply-loop sampler task.
  • The source WAL head query in etl-postgres/src/lag.rs (flushed WAL on a primary, replayed WAL on a standby) and the matching docs changes.
Extended reasoning...

The change spans 16 files (about 1000 lines added, 485 removed). It replaces the per-apply-loop lag sampler with a single pipeline-level sampler and new etl-postgres queries for the source WAL head. It removes five Prometheus gauges and adds two new ones plus a pipeline_id label, which breaks existing dashboards and alerts. It touches no security-sensitive surface such as auth, injection or data exposure. The diff is large, it changes the meaning of the metrics, and .github/CODEOWNERS assigns every path to the pipelines team, so this was deferred rather than approved.

@iambriccardo
iambriccardo merged commit 4cc6d00 into main Oct 17, 2026
22 checks passed
@iambriccardo
iambriccardo deleted the fix-lsn-check branch October 17, 2026 16:37
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.

2 participants