Hi Bruno, Thanks for the KIP, this will be a useful addition.
Overall the KIP looks good to me, and I have two minor comments. 1. For the tags should, I'm wondering if rocksdb-state-id should be rocksdb-store-id instead? 2. With the compaction metrics, would it be possible to add total compactions and an average number of compactions? I've taken a look at the available RocksDB metrics, and I'm not sure. But users can control how many L0 files it takes to trigger compaction so if it is possible; it may be useful. Thanks, Bill On Mon, May 20, 2019 at 9:15 AM Bruno Cadonna <br...@confluent.io> wrote: > Hi Sophie, > > Thank you for your comments. > > It's a good idea to supplement the metrics with configuration option > to change the metrics. I also had some thoughts about it. However, I > think I need some experimentation to get this right. > > I added the block cache hit rates for index and filter blocks to the > KIP. As far as I understood, they should stay at zero, if users do not > configure RocksDB to include index and filter blocks into the block > cache. Did you also understand this similarly? I guess also in this > case some experimentation would be good to be sure. > > Best, > Bruno > > > On Sat, May 18, 2019 at 2:29 AM Sophie Blee-Goldman <sop...@confluent.io> > wrote: > > > > Actually I wonder if it might be useful to users to be able to break up > the > > cache hit stats by type? Some people may choose to store index and filter > > blocks alongside data blocks, and it would probably be very helpful for > > them to know who is making more effective use of the cache in order to > tune > > how much of it is allocated to each. I'm not sure how common this really > is > > but I think it would be invaluable to those who do. RocksDB performance > can > > be quite opaque.. > > > > Cheers, > > Sophie > > > > On Fri, May 17, 2019 at 5:01 PM Sophie Blee-Goldman <sop...@confluent.io > > > > wrote: > > > > > Hey Bruno! > > > > > > This all looks pretty good to me, but one suggestion I have is to > > > supplement each of the metrics with some info on how the user can > control > > > them. In other words, which options could/should they set in > > > RocksDBConfigSetter should they discover a particular bottleneck? > > > > > > I don't think this necessarily needs to go into the KIP, but I do > think it > > > should be included in the docs somewhere (happy to help build up the > list > > > of associated options when the time comes) > > > > > > Thanks! > > > Sophie > > > > > > On Fri, May 17, 2019 at 2:54 PM Bruno Cadonna <br...@confluent.io> > wrote: > > > > > >> Hi all, > > >> > > >> this KIP describes the extension of the Kafka Streams' metrics to > include > > >> RocksDB's internal statistics. > > >> > > >> Please have a look at it and let me know what you think. Since I am > not a > > >> RocksDB expert, I am thankful for any additional pair of eyes that > > >> evaluates this KIP. > > >> > > >> > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-471:+Expose+RocksDB+Metrics+in+Kafka+Streams > > >> > > >> Best regards, > > >> Bruno > > >> > > > >
