mjsax commented on code in PR #18771:
URL: https://github.com/apache/kafka/pull/18771#discussion_r1938336028
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java:
##########
@@ -156,11 +156,8 @@ private void registerMetrics() {
(config, now) -> numOpenIterators.sum());
StateStoreMetrics.addOldestOpenIteratorGauge(taskId.toString(),
metricsScope, name(), streamsMetrics,
(config, now) -> {
- try {
- return openIterators.isEmpty() ? null :
openIterators.first().startTimestamp();
- } catch (final NoSuchElementException ignored) {
- return null;
- }
+ final Iterator<MeteredIterator> openIteratorsIterator =
openIterators.iterator();
+ return openIteratorsIterator.hasNext() ?
openIteratorsIterator.next().startTimestamp() : null;
Review Comment:
Thanks for digging JavaDocs. Did not have a close looker earlier. I you are
right, it seems that
> they are guaranteed to traverse elements as they existed upon construction
exactly once, and may (but are not guaranteed to) reflect any modifications
subsequent to construction.
Is the key statement. I was never worried about
`ConcurrentModificationException`, but only about `NoSuchElementException`.
Maybe @nicktelford, who did the original PR can help out on this question,
just to double check.
Btw: if we go forward with this PR, we need to also apply this change to
`MeteredWindowedStore` and `MeteredSessionStore`.
--
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]
