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 possible to identify a few very long-running / leaked iterators? I
can imagine on a busy node, it would be hard to spot that 1% of the
iterators never close when looking only at closed iterator or the
number of iterators. But it could still be good to identify those
leaks early. One option would be to add `iterator-duration-max` and
take open iterators into account when computing the metric.
Cheers,
Lucas
On Thu, Oct 5, 2023 at 3:50 PM Nick Telford <nick.telf...@gmail.com> wrote:
>
> 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<Bytes, byte[]>
> rocksDbPrefixSeekIterator = cf.prefixScan(accessor, prefixBytes);
> --> final long startedAt = System.nanoTime();
> openIterators.add(rocksDbPrefixSeekIterator);
> rocksDbPrefixSeekIterator.onClose(() -> {
> --> metricsRecorder.recordIteratorDuration(System.nanoTime() -
> startedAt);
> openIterators.remove(rocksDbPrefixSeekIterator);
> });
>
> The lines with the arrow are the new code. This pattern is repeated
> throughout RocksDBStore, wherever a new RocksDbIterator is created.
>
> Regards,
> Nick
>
> On Thu, 5 Oct 2023 at 12:32, Colt McNealy <c...@littlehorse.io> wrote:
>
> > 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 whenever an Iterator's
> > close() method is called
> >
> > Does the Iterator have its own "createdAt()" or equivalent field, or do we
> > need to keep track of the Iterator's start time upon creation?
> >
> > Cheers,
> > Colt McNealy
> >
> > *Founder, LittleHorse.dev*
> >
> >
> > On Wed, Oct 4, 2023 at 9:07 AM Nick Telford <nick.telf...@gmail.com>
> > wrote:
> >
> > > 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!
> > >
> > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-989%3A+RocksDB+Iterator+Metrics
> > >
> > > P.S. I'm not too sure about the formatting of the "New Metrics" table,
> > any
> > > advice there would be appreciated.
> > >
> > > Regards,
> > > Nick
> > >
> >
