lhotari opened a new pull request, #4774: URL: https://github.com/apache/bookkeeper/pull/4774
### Motivation BookKeeper still depends on `com.yahoo.datasketches:sketches-core:0.8.3`, which has been unmaintained since the project moved to the Apache Software Foundation and was renamed to `org.apache.datasketches:datasketches-java`. A previous attempt to upgrade ([#3264](https://github.com/apache/bookkeeper/pull/3264), May 2022) replaced the dependency 1:1 with the Apache classic-quantiles `DoublesSketch` but never landed. Since #3264 two things have changed: 1. BookKeeper master now requires Java 17, so we can use the latest `datasketches-java:7.0.1` (which itself requires JDK ≥ 17). 2. The Apache DataSketches maintainers explicitly recommend the **KLL sketch** over the classic quantiles sketch for any new code — see https://github.com/apache/datasketches-java/issues/398#issuecomment-1151490051. KLL is faster, more memory efficient, and has comparable accuracy at the default K. Migrating straight to KLL while we are touching this code avoids the near-term performance regression that #3264 would have introduced. ### Changes - `pom.xml` and `stats/bookkeeper-stats-providers/prometheus-metrics-provider/pom.xml`: dependency coordinate updated from `com.yahoo.datasketches:sketches-core:0.8.3` → `org.apache.datasketches:datasketches-java:7.0.1`. - `DataSketchesOpStatsLogger.java`: ported to `org.apache.datasketches.kll.KllDoublesSketch`. - On-heap throughout, matching the previous behavior — no off-heap conversion. - KLL has no `DoublesUnion`; rotation aggregates with `merge()` directly into a freshly allocated `KllDoublesSketch.newHeapInstance()`. - KLL has no `reset()`; per-thread sketches are reassigned to a fresh `newHeapInstance()` under the existing `StampedLock` write lock (the lock provides the happens-before edge that publishes the new reference safely). - `KllDoublesSketch.getQuantile()` throws `SketchesArgumentException` on an empty sketch (vs the classic sketch returning `NaN`), so `getQuantileValue()` now guards with `isEmpty()` to preserve the prior `NaN` return. - `LICENSE-all.bin.txt` / `LICENSE-server.bin.txt`: list `datasketches-java-7.0.1.jar` and the transitively-required `datasketches-memory-4.1.0.jar` separately. The two jars are sourced from distinct Apache repositories (`apache/datasketches-java` and `apache/datasketches-memory`), so each gets its own footnote — `[25]` retargeted to datasketches-java, and the previously-vacant `[27]` slot now references datasketches-memory. The CVE-2021-40531 OWASP suppression that #3264 added does **not** apply to 7.0.x, so no suppression is needed. ### Verification - `mvn -pl stats/bookkeeper-stats-providers/prometheus-metrics-provider -am dependency:tree -Dincludes=org.apache.datasketches` confirms the resolved versions: `datasketches-java:jar:7.0.1:compile` → `datasketches-memory:jar:4.1.0:compile` (matches the LICENSE entries). - `mvn -pl stats/bookkeeper-stats-providers/prometheus-metrics-provider -am clean verify` builds green; all 11 tests in `TestPrometheusFormatter` and `TestPrometheusMetricsProvider` pass. - `mvn -pl stats/bookkeeper-stats-providers/prometheus-metrics-provider -am spotbugs:check -DskipTests`: no errors/warnings. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
