nicktelford commented on code in PR #18771:
URL: https://github.com/apache/kafka/pull/18771#discussion_r1938365936
##########
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:
> Also talked to @ijuma about it, and he made the good point that
`hasNext/next` must provide the guarantee to not throw
`NoSuchElementException`, otherwise every multi-threaded "foreach" would be
broken on a concurrent collection.
Yeah, that's what I've been thinking over the last few hours too. This is
why I think checking the implementation for clarification wouldn't be terrible,
because it would likely clarify the guarantees they have documented.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
