[automatic failover] Implement sliding time window metrics tracker #3521

ggivo · 2025-11-19T14:40:54Z

PR Description

Summary

Implements sliding time window metrics tracker for automatic failover by porting Resilience4j's LockFreeSlidingTimeWindowMetrics.

Problem

The initial POC implementation didn't pre-aggregate metrics, causing performance degradation as the time window increased.

Solution

Ported LockFreeSlidingTimeWindowMetrics from Resilience4j with modifications:
- Java 8 compatibility: Replaced VarHandle with AtomicReference
- Stripped down to track only success/failure counts (removed duration and slow call tracking)
- Lock-free implementation using CAS for thread-safe concurrent access

Changes

Replaced LockFreeSlidingWindowMetrics implementation with lock-free reselience4j port
Updated CircuitBreakerMetricsImpl to use new metrics implementation

Bug Fixes: - Fix: Ensure snapshot metrics remain accurate after a full window rotation - Fix: events recorded exactly at bucket boundaries were miscounted - Enforce window size % bucket size == 0 - Move LockFreeSlidingWindowMetricsUnitTests to correct package (io.lettuce.core.failover.metrics)

- remove snapshotTime - not used & not correctly calcualted - remove reset metrics - unused as of now

ggivo · 2025-11-19T14:57:46Z

I believe failover metrics-related classes should not be part of the publicly supported API
Should we move them into io.lettuce.core.failover.internal.metrics or mark them internal in any other way?
@tishun @atakavci

Copilot

Pull Request Overview

This PR implements a lock-free sliding time window metrics tracker for automatic failover by porting Resilience4j's LockFreeSlidingTimeWindowMetrics. The implementation replaces the previous POC version that didn't pre-aggregate metrics, which caused performance issues as the time window increased.

Key changes:

Ported LockFreeSlidingTimeWindowMetrics from Resilience4j with Java 8 compatibility (using AtomicReferenceFieldUpdater instead of VarHandle)
Removed the old LockFreeSlidingWindowMetrics and TimeWindowBucket classes
Simplified the API by removing the reset() method from metrics interfaces
Converted MetricsSnapshot from a concrete class to an interface with MetricsSnapshotImpl as implementation

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
src/main/java/io/lettuce/core/failover/metrics/LockFreeSlidingTimeWindowMetrics.java	New lock-free sliding window implementation ported from Resilience4j using linked list of time slices
src/main/java/io/lettuce/core/failover/metrics/PackedAggregation.java	Cache-friendly measurement implementation for tracking call counts and outcomes
src/main/java/io/lettuce/core/failover/metrics/Outcome.java	Simple enum for SUCCESS/FAILURE outcomes
src/main/java/io/lettuce/core/failover/metrics/CumulativeMeasurement.java	Interface for measurements that accumulate call outcomes
src/main/java/io/lettuce/core/failover/metrics/MeasurementData.java	Interface for accessing measurement data
src/main/java/io/lettuce/core/failover/metrics/Clock.java	Clock abstraction for testable time-dependent code
src/main/java/io/lettuce/core/failover/metrics/MetricsSnapshot.java	Converted from concrete class to interface
src/main/java/io/lettuce/core/failover/metrics/MetricsSnapshotImpl.java	New implementation of MetricsSnapshot interface
src/main/java/io/lettuce/core/failover/metrics/CircuitBreakerMetricsImpl.java	Updated to use new LockFreeSlidingTimeWindowMetrics with seconds-based window configuration
src/main/java/io/lettuce/core/failover/metrics/CircuitBreakerMetrics.java	Removed reset() method from interface
src/main/java/io/lettuce/core/failover/metrics/SlidingWindowMetrics.java	Removed reset() method from interface
src/main/java/io/lettuce/core/failover/metrics/LockFreeSlidingWindowMetrics.java	Removed old implementation
src/main/java/io/lettuce/core/failover/metrics/TimeWindowBucket.java	Removed old bucket implementation
src/test/java/io/lettuce/core/failover/metrics/TestClock.java	New controllable clock implementation for testing
src/test/java/io/lettuce/core/failover/metrics/SlidingWindowMetricsUnitTests.java	New comprehensive unit tests for the sliding window implementation
src/test/java/io/lettuce/core/failover/metrics/SlidingWindowMetricsPerformanceTests.java	Updated performance tests to use new implementation
src/test/java/io/lettuce/core/failover/LockFreeSlidingWindowMetricsUnitTests.java	Removed old unit tests
src/test/jmh/io/lettuce/core/failover/metrics/FailoverMetricsBenchmark.java	New JMH benchmark for metrics performance testing
src/test/jmh/io/lettuce/core/failover/metrics/JmhMain.java	New JMH test launcher for manual benchmark execution

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

src/test/java/io/lettuce/core/failover/metrics/TestClock.java

src/test/jmh/io/lettuce/core/failover/metrics/FailoverMetricsBenchmark.java

src/main/java/io/lettuce/core/failover/metrics/Clock.java

src/test/java/io/lettuce/core/failover/metrics/TestClock.java

src/main/java/io/lettuce/core/failover/metrics/CircuitBreakerMetricsImpl.java

src/main/java/io/lettuce/core/failover/metrics/Clock.java

src/test/jmh/io/lettuce/core/failover/metrics/FailoverMetricsBenchmark.java

src/main/java/io/lettuce/core/failover/metrics/LockFreeSlidingTimeWindowMetrics.java

src/test/java/io/lettuce/core/failover/metrics/TestClock.java

atakavci · 2025-11-19T15:17:22Z

I believe failover metrics-related classes should not be part of the publicly supported API Should we move them into io.lettuce.core.failover.internal.metrics or mark them internal in any other way? @tishun @atakavci

Lets make MetricsSnapshot public,, for the rest it is package private.
To do that, all needs to move to failover package, which is fine to me.
OR
We can leave them as public types, and expose/handover only interfaces rather than concrete instances from our API.

TBH, i dont like that internals territory at all.

- fix incorrect javadoc - fix failing benchmark

- CircuitBreakerMetrics, MetricsSnapshot - public - metrics implementation details stay inside io.lettuce.core.failover.metrics - Update CircuitBreaker to obtain its metrics via CircuitBreakerMetricsFactory.createLockFree()

ggivo · 2025-11-20T13:56:29Z

@atakavci

Lets make MetricsSnapshot public, for the rest it is package private.

To avoid including metrics in the parent failover package, I kept the classes package private under **failover. metrics
Kept public MetricsSnapshot & CircuitBreakerMetrics interfaces and introduced a factory for creating the concrete implementations

Copilot

Pull Request Overview

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

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

src/main/java/io/lettuce/core/failover/metrics/Clock.java

src/main/java/io/lettuce/core/failover/metrics/PackedAggregation.java

Co-authored-by: Copilot <[email protected]>

atakavci

Thanks for investing time on performance @ggivo , this is as good as it gets in the time frame.
Just left a couple of comments.. The only interesting one would be the one with reset.

atakavci · 2025-11-20T17:59:07Z

src/main/java/io/lettuce/core/failover/metrics/CircuitBreakerMetrics.java

-    /**
-     * Reset all metrics to zero.
-     */
-    void reset();


There might be cases where the CB transitions into OPEN and CLOSED states in a quick fashion which can fall into the configured window size. Though we will have grace periods and health check cycles, this might be useful to make sure when multiple transitions happen close enough to risk the CB behaviour.

@atakavci
Yes, came to the same conclusion reviewing other PR's,
I was thinking of recreating the Metrics inside CircuiteBreaker, but it makes more sense to delegate to Metrics implementations to handle the reset.
Will work on implementing it back!

@atakavci
Looking at the code, I think it will be easier and safer to implement the reset at the CircuitBreaker level.
e.g, CircuitBreakerMetrics are immutable, and when we call reset inside CircuitBreaker, we create a new instance.

Also considering opening a dedicated PR for this, since this one is starting to become hard for review

At the same time started to question if we need to reset the metrics when transitioning between OPEN, and CLOSED states, or if we should preserve them since it is a moving window, and calls made during that period are valid.

Let's discuss.

src/main/java/io/lettuce/core/failover/metrics/CircuitBreakerMetricsFactory.java

src/main/java/io/lettuce/core/failover/metrics/SlidingWindowMetrics.java

- remove CircuitBreakerMetrics, CircuitBreakerMetricsImpl - rename SlidingWindowMetrics -> CircuitBreakerMetrics

Copilot

Pull Request Overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 13 comments.

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

src/main/java/io/lettuce/core/failover/CircuitBreaker.java

src/main/java/io/lettuce/core/failover/metrics/LockFreeSlidingTimeWindowMetrics.java

src/main/java/io/lettuce/core/failover/metrics/MeasurementData.java

src/main/java/io/lettuce/core/failover/metrics/Clock.java

src/main/java/io/lettuce/core/failover/metrics/LockFreeSlidingTimeWindowMetrics.java

src/main/java/io/lettuce/core/failover/metrics/PackedAggregation.java

src/main/java/io/lettuce/core/failover/metrics/MetricsFactory.java

src/main/java/io/lettuce/core/failover/metrics/PackedAggregation.java

src/main/java/io/lettuce/core/failover/metrics/Outcome.java

ggivo added 9 commits November 18, 2025 15:08

abstract clock for easy testing

1dcd5a7

remove unused reset methods

ff7e989

extract interface for MetricsSnapshot

9223954

- remove snapshotTime - not used & not correctly calcualted - remove reset metrics - unused as of now

add LockFreeSlidingWindowMetrics benchmark test

eda5f6e

performance tests moved to metrics package

ebbb006

replace with port from reselience4j

bb104fe

update copyrights

d78bacb

format

30f3247

ggivo changed the title ~~[automatic failover][lettuce] Implement sliding time window metrics tracker~~ [automatic failover] Implement sliding time window metrics tracker Nov 19, 2025

clean up javadocs

3f11b49

ggivo requested a review from Copilot November 19, 2025 14:55

Copilot started reviewing on behalf of ggivo November 19, 2025 14:56 View session

Copilot finished reviewing on behalf of ggivo November 19, 2025 15:00

Copilot AI reviewed Nov 19, 2025

View reviewed changes

ggivo added 2 commits November 20, 2025 15:04

clean up

e73afdc

- fix incorrect javadoc - fix failing benchmark

[automatic failover] Hide failover metrics implementation

8c9dd2c

- CircuitBreakerMetrics, MetricsSnapshot - public - metrics implementation details stay inside io.lettuce.core.failover.metrics - Update CircuitBreaker to obtain its metrics via CircuitBreakerMetricsFactory.createLockFree()

ggivo requested a review from Copilot November 20, 2025 13:56

Copilot started reviewing on behalf of ggivo November 20, 2025 13:57 View session

Copilot finished reviewing on behalf of ggivo November 20, 2025 14:00

Copilot AI reviewed Nov 20, 2025

View reviewed changes

src/main/java/io/lettuce/core/failover/metrics/Clock.java Outdated Show resolved Hide resolved

src/main/java/io/lettuce/core/failover/metrics/PackedAggregation.java Outdated Show resolved Hide resolved

Apply suggestions from code review

5e93c62

Co-authored-by: Copilot <[email protected]>

atakavci reviewed Nov 21, 2025

View reviewed changes

ggivo added 2 commits November 21, 2025 14:05

rename createLockFree -> createDefaultMetrics

9064266

address review comments by @atakavci

f190663

- remove CircuitBreakerMetrics, CircuitBreakerMetricsImpl - rename SlidingWindowMetrics -> CircuitBreakerMetrics

ggivo requested review from Copilot and tishun November 21, 2025 13:15

ggivo requested a review from a-TODO-rov November 21, 2025 13:15

Copilot started reviewing on behalf of ggivo November 21, 2025 13:15 View session

format

9aecc79

Copilot finished reviewing on behalf of ggivo November 21, 2025 13:20

Copilot AI reviewed Nov 21, 2025

View reviewed changes

[automatic failover] Implement sliding time window metrics tracker #3521

Are you sure you want to change the base?

[automatic failover] Implement sliding time window metrics tracker #3521

Uh oh!

Conversation

ggivo commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

Summary

Problem

Solution

Changes

Uh oh!

ggivo commented Nov 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

atakavci commented Nov 19, 2025

Uh oh!

ggivo commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

atakavci left a comment

Choose a reason for hiding this comment

Uh oh!

atakavci Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

ggivo Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggivo Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ggivo commented Nov 19, 2025 •

edited

Loading

ggivo commented Nov 20, 2025 •

edited

Loading

ggivo Nov 21, 2025 •

edited

Loading