Idle time should not depend on metrics recorder

[violetagg]

Expected behavior

Idle time should not depend on metrics recorder

Actual behavior

The current implementation uses metrics recorder to calculate the idle time

void markReleased() {
    if (STATE.compareAndSet(this, STATE_ACQUIRED, STATE_RELEASED)) {
        this.timeSinceRelease = metricsRecorder.now();
    }
}

When there is no metrics recorder reactor.pool.NoOpPoolMetricsRecorder is used
where now() and measureTime(long startTimeMillis) always return 0

This change works for me

diff --git a/src/main/java/reactor/pool/NoOpPoolMetricsRecorder.java b/src/main/java/reactor/pool/NoOpPoolMetricsRecorder.java
index 532b4f8..419f7b6 100644
--- a/src/main/java/reactor/pool/NoOpPoolMetricsRecorder.java
+++ b/src/main/java/reactor/pool/NoOpPoolMetricsRecorder.java
@@ -30,12 +30,7 @@ final class NoOpPoolMetricsRecorder implements PoolMetricsRecorder {
 
     @Override
     public long now() {
-        return 0L;
-    }
-
-    @Override
-    public long measureTime(long startTimeMillis) {
-        return 0;
+        return System.currentTimeMillis();
     }
 
     @Override
diff --git a/src/main/java/reactor/pool/PoolMetricsRecorder.java b/src/main/java/reactor/pool/PoolMetricsRecorder.java
index 437cefb..37bd741 100644
--- a/src/main/java/reactor/pool/PoolMetricsRecorder.java
+++ b/src/main/java/reactor/pool/PoolMetricsRecorder.java
@@ -32,7 +32,9 @@ public interface PoolMetricsRecorder {
 	 * @param startTimeMillis the starting time initially obtained via {@link #now()}
 	 * @return the elapsed time in milliseconds
 	 */
-	long measureTime(long startTimeMillis);
+	default long measureTime(long startTimeMillis) {
+		return now() - startTimeMillis;
+	}
 
 	/**
 	 * Get a starting time with milliseconds resolution.

Steps to reproduce

Reactor Pool version

current snapshot version

JVM version (e.g. java -version)

OS version (e.g. uname -a)

[simonbasle]

The concern between exporting the metrics and timestamping/measuring a timing should be separated indeed, since the later is used on the PooledRef for stuff like eviction, as you found out.

This is a bit last minute, but I think it is worth the effort to split that concern and have a Clock option on the PoolBuilder for that purpose, along with removing the now() and measureTime methods on PoolMetricsRecorder.

cc @mp911de for r2dbc-pool