ableegoldman commented on code in PR #17713:
URL: https://github.com/apache/kafka/pull/17713#discussion_r1849779903
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java:
##########
@@ -154,7 +155,13 @@ private void registerMetrics() {
StateStoreMetrics.addNumOpenIteratorsGauge(taskId.toString(),
metricsScope, name(), streamsMetrics,
(config, now) -> numOpenIterators.sum());
StateStoreMetrics.addOldestOpenIteratorGauge(taskId.toString(),
metricsScope, name(), streamsMetrics,
- (config, now) -> openIterators.isEmpty() ? null :
openIterators.first().startTimestamp()
+ (config, now) -> {
+ try {
+ return openIterators.isEmpty() ? null :
openIterators.first().startTimestamp();
Review Comment:
> Unfortunately, the internals of ConcurrentSkipListSet are private, so our
own version would need to be a complete copy of the JDK implementation. This is
probably not a good idea, as upgrading JDK would not benefit from any
improvements in the JDK implementation.
I actually think this wouldn't be so bad, since as you pointed out, the
ConcurrentSkipListSet is basically a no-op wrapper around the
ConcurrentSkipListMap which is where all the messy stuff happens. You could
more or less copy the existing ConcurrentSkipListSet implementation as-is and
just change the name (and add the `#firstEntry` API we need), it truly does
nothing but delegate to the underlying wrap. So I can't imagine there would be
anything to change, much less improve, in the JDK implementation of
ConcurrentSkipListSet
Just wanted to clarify, as I said before I'm actually fine with the way it's
implemented now so I'm not asking for anything to be changed. But if we ever do
want to change this or run into a similar problem, imo writing our own wrapper
around the ConcurrentSkipListMap is the way to go 🤷♀️
--
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]
