SGTM to use nano-seconds across the board.
On 5/16/24 7:12 AM, Nick Telford wrote:
Actually, one other point: our existing state store operation metrics are
measured in nanoseconds[1].
Should iterator-duration-(avg|max) also be measured in nanoseconds, for
consistency, or should we keep them
Actually, one other point: our existing state store operation metrics are
measured in nanoseconds[1].
Should iterator-duration-(avg|max) also be measured in nanoseconds, for
consistency, or should we keep them milliseconds, as the KIP currently
states?
1:
Good point! I've updated it to "Improved StateStore Iterator metrics for
detecting leaks" - let me know if you have a better suggestion.
This should affect the voting imo, as nothing of substance has changed.
Regards,
Nick
On Thu, 16 May 2024 at 01:39, Sophie Blee-Goldman
wrote:
> One quick
One quick thing -- can you update the title of this KIP to reflect the
decision to implement these metrics for all state stores implementations
rather than just RocksDB?
On Tue, May 14, 2024 at 1:36 PM Nick Telford wrote:
> Woops! Thanks for the catch Lucas. Given this was just a typo, I don't
Woops! Thanks for the catch Lucas. Given this was just a typo, I don't
think this affects the voting.
Cheers,
Nick
On Tue, 14 May 2024 at 18:06, Lucas Brutschy
wrote:
> Hi Nick,
>
> you are still referring to oldest-open-iterator-age-ms in the
> `Proposed Changes` section.
>
> Cheers,
> Lucas
Hi Nick,
you are still referring to oldest-open-iterator-age-ms in the
`Proposed Changes` section.
Cheers,
Lucas
On Thu, May 2, 2024 at 4:00 PM Lucas Brutschy wrote:
>
> Hi Nick!
>
> I agree, the age variant is a bit nicer since the semantics are very
> clear from the name. If you'd rather go
Hi Nick!
I agree, the age variant is a bit nicer since the semantics are very
clear from the name. If you'd rather go for the simple implementation,
how about calling it `oldest-iterator-open-since-ms`? I believe this
could be understood without docs. Either way, I think we should be
able to open
Hi Matthias,
> For the oldest iterator metric, I would propose something simple like
> `iterator-opened-ms` and it would just be the actual timestamp when the
> iterator was opened. I don't think we need to compute the actual age,
> but user can to this computation themselves?
That works for me;
The time window thing was just an idea. Happy to drop it.
For the oldest iterator metric, I would propose something simple like
`iterator-opened-ms` and it would just be the actual timestamp when the
iterator was opened. I don't think we need to compute the actual age,
but user can to this
Quick addendum:
My suggested metric "oldest-open-iterator-age-seconds" should be
"oldest-open-iterator-age-ms". Milliseconds is obviously a better
granularity for such a metric.
Still accepting suggestions for a better name.
On Thu, 28 Mar 2024 at 13:41, Nick Telford wrote:
> Hi everyone,
>
>
Hi everyone,
Sorry for leaving this for so long. So much for "3 weeks until KIP freeze"!
On Sophie's comments:
1. Would Matthias's suggestion of a separate metric tracking the age of the
oldest open iterator (within the tag set) satisfy this? That way we can
keep iterator-duration-(avg|max) for
About your last two points: I completely agree that we should try to
make this independent of RocksDB, and should probably adopt a
general philosophy of being store-implementation agnostic unless
there is good reason to focus on a particular store type: eg if it was
only possible to implement for
Seems I am late to this party. Can we pick this up again aiming for 3.8
release? I think it would be a great addition. Few comments:
- I think it does make sense to report `iterator-duration-avg` and
`iterator-duration-max` for all *closed* iterators -- it just seems to
be a useful metric
>
> If we used "iterator-duration-max", for
> example, would it not be confusing that it includes Iterators that are
> still open, and therefore the duration is not yet known?
1. Ah, I think I understand your concern better now -- I totally agree that
a
"iterator-duration-max" metric would be
I don't really have a problem with adding such a metric, I'm just not
entirely sure how it would work. If we used "iterator-duration-max", for
example, would it not be confusing that it includes Iterators that are
still open, and therefore the duration is not yet known? When graphing that
over
I actually think we could implement Lucas' suggestion pretty easily and
without too much additional effort. We have full control over the iterator
that is returned by the various range queries, so it would be easy to
register a gauge metric for how long it has been since the iterator was
created.
Hi Nick,
I did not think in detail about how to implement it, just about what
metrics would be nice to have. You are right, we'd have to
register/deregister the iterators during open/close. This would be
more complicated to implement than the other metrics, but I do not see
a fundamental problem
Hi Lucas,
Hmm, I'm not sure how we could reliably identify such leaked Iterators. If
we tried to include open iterators when calculating iterator-duration, we'd
need some kind of registry of all the open iterator creation timestamps,
wouldn't we?
In general, if you have a leaky Iterator, it
Hi Nick!
thanks for the KIP! I think this could be quite useful, given the
problems that we had with leaks due to RocksDB resources not being
closed.
I don't have any pressing issues why we can't accept it like it is,
just one minor point for discussion: would it also make sense to make
it
Hi Colt,
I kept the details out of the KIP, because KIPs generally document
high-level design, but the way I'm doing it is like this:
final ManagedKeyValueIterator
rocksDbPrefixSeekIterator = cf.prefixScan(accessor, prefixBytes);
--> final long startedAt = System.nanoTime();
Thank you for the KIP, Nick!
This would be highly useful for many reasons. Much more sane than checking
for leaked iterators by profiling memory usage while running 100's of
thousands of range scans via interactive queries (:
One small question:
>The iterator-duration metrics will be updated
Hi everyone,
KIP-989 is a small Kafka Streams KIP to add a few new metrics around the
creation and use of RocksDB Iterators, to aid users in identifying
"Iterator leaks" that could cause applications to leak native memory.
Let me know what you think!
22 matches
Mail list logo