Skip to content

perf: coalesce ring heartbeat ticks at one second - #252

Merged
ovaistariq merged 2 commits into
mainfrom
perfloop-pr-open-5g7prhpq1a
Sep 9, 2026
Merged

ovaistariq merged 2 commits into
mainfrom
perfloop-pr-open-5g7prhpq1a

Conversation

@perfloop

@perfloop perfloop Bot commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Ring timestamps use Unix-second precision, so the 500 ms heartbeat default can retry a CAS without changing membership. Each retry also repeats the heartbeat delegate work.

Set the default and minimum effective ring heartbeat period to one second. Sub-second configurations are intentionally clamped because the stored timestamp cannot represent that cadence. Longer configured periods remain unchanged, the existing 60-second heartbeat-timeout floor remains in place, and state transitions continue to use their direct path. The benchmark captures its measurement endpoint before stopping the lifecycler and keeps the seeded instance in the ring during teardown, so shutdown CAS operations stay outside the heartbeat metrics.

Workload: single local memberlist ring lifecycler heartbeat with 8 seeded members, 1 token, 100ms sampling steps, and a 10s benchmark window

Workload: single local memberlist ring lifecycler heartbeat with 8 seeded members, 1 token, 100ms sampling steps, and a 10s benchmark window

Metric Before After Change
cas-attempts/heartbeat 2.222 1 55% lower
heartbeat-callbacks/heartbeat 2.111 1 52.6% lower
Checks: 1 passed.

Tests:

  • make test-coordinator
  • make test-integration-coordinator
  • make lint
  • make license-check

Generated by Perfloop. Measurements and checks.


Note

Medium Risk
Changes cluster liveness update frequency and clamps custom sub-second heartbeat_period values, which can slightly delay ring timestamp updates but reduces KV/gossip load; failure detection still governed by the 60s timeout floor.

Overview
Aligns ring heartbeat cadence with one-second timestamp precision in the KV ring: default DefaultHeartbeatPeriod moves from 500ms to 1s, and MinHeartbeatPeriod plus effectiveHeartbeatPeriod clamp sub-second settings in ApplyDefaults and ToBasicLifecyclerConfig so dskit no longer ticks faster than stored timestamps can change—cutting redundant memberlist CAS attempts and duplicate heartbeat delegate work (benchmark reports ~1 CAS attempt and ~1 callback per successful heartbeat vs ~2.2 before).

Adds BenchmarkRingHeartbeat and make build-bench-ring-heartbeat for Perfloop-style precompiled runs, with metrics captured before lifecycler teardown so shutdown CAS is excluded. Tests cover sub-second flooring; cluster and RFC-007 docs now describe the 1s minimum and coalescing behavior.

Reviewed by Cursor Bugbot for commit f7db423. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Greptile Summary

The PR coalesces ring heartbeat periods to a one-second minimum, matching ring timestamp precision and avoiding redundant CAS and delegate work.

  • Raises the default heartbeat period from 500 ms to one second and clamps sub-second configurations.
  • Adds configuration coverage and a dedicated memberlist heartbeat benchmark.
  • Captures benchmark metrics before teardown and keeps the seeded instance registered during shutdown.
  • Updates cluster documentation and the sharded-cluster RFC.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
coordinator/ring/config.go Centralizes the one-second effective heartbeat floor and applies it during both defaulting and direct dskit conversion.
coordinator/ring/config_test.go Covers sub-second normalization and verifies direct conversion clamps without mutating the source configuration.
coordinator/ring/heartbeat_benchmark_test.go Adds heartbeat CAS/callback measurements and correctly excludes shutdown lifecycle work from the final snapshot.
Makefile Adds a target for compiling the ring heartbeat benchmark.
docs/cluster.md Documents the one-second default and sub-second coalescing behavior.
docs/rfcs/RFC-007-sharded-cluster.md Updates the ring heartbeat design documentation to reflect the one-second minimum.

Reviews (2): Last reviewed commit: "perf: coalesce ring heartbeats at one se..." | Re-trigger Greptile

Comment thread coordinator/ring/heartbeat_benchmark_test.go Outdated
@perfloop
perfloop Bot force-pushed the perfloop-pr-open-5g7prhpq1a branch from ce7ab1b to 604f41e Compare September 2, 2026 10:39
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
@ovaistariq
ovaistariq force-pushed the perfloop-pr-open-5g7prhpq1a branch from 604f41e to f7db423 Compare September 9, 2026 22:45
@ovaistariq

Copy link
Copy Markdown
Contributor

Rebased onto main (f7db423, no code change). The earlier build failure was the eviction e2e flake fixed on main in #261, not this change. Coordinator suites pass. Ready to merge.

@ovaistariq
ovaistariq merged commit 4843daf into main Sep 9, 2026
8 checks passed
@ovaistariq
ovaistariq deleted the perfloop-pr-open-5g7prhpq1a branch September 9, 2026 23:32
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