Conversation
We can remove the explicit counter for open iterators, and just use size() on the set of open iterators we track anyway.
|
Follow up PR to #20022 (comment) |
| numOpenIterators.increment(); | ||
|
|
||
| if (numOpenIterators.intValue() == 1) { | ||
| if (openIterators.size() == 1) { |
There was a problem hiding this comment.
I was worried for adding the same iterator, which could create unexpected metrics, but it seems StateStoreMetrics.addOldestOpenIteratorGauge already handle this case.
There was a problem hiding this comment.
Not sue if I can follow. Can you elaborate?
There was a problem hiding this comment.
Sorry for the unclear comment. Please see the following example
var openIterators = new OpenIterators();
var iterator = new MeteredIterator(1000L);
openIterators.add(iterator);
openIterators.add(iterator);The second openIterators.add(iterator) did not invoke StateStoreMetrics.addOldestOpenIteratorGauge before, but it does now. However, this should be fine, since StateStoreMetrics.addOldestOpenIteratorGauge does not create new metrics in this scenario.
There was a problem hiding this comment.
Well, would be a bug anyway, to call openIterators.add(iterator); twice on the same iterator object. Each iterator should be registered exactly one time.
But yes, we would not add a new metric.
We can remove the explicit counter for open iterators, and just use size() on the set of open iterators we track anyway. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
We can remove the explicit counter for open iterators, and just use size() on the set of open iterators we track anyway. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
We can remove the explicit counter for open iterators, and just use size() on the set of open iterators we track anyway. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
2 participants
We can remove the explicit counter for open iterators, and just use
size() on the set of open iterators we track anyway.
Reviewers: Chia-Ping Tsai chia7712@gmail.com