[GitHub] [kafka] nicktelford commented on pull request #14317: KAFKA-13973: Fix inflated block cache metrics
nicktelford commented on PR #14317: URL: https://github.com/apache/kafka/pull/14317#issuecomment-1715792093 @cadonna I've added a test now that I think tests this suitably. Crucially, the test fails without this fix applied, and passes with this fix applied, as you would expect. -- 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
[GitHub] [kafka] nicktelford commented on pull request #14317: KAFKA-13973: Fix inflated block cache metrics
nicktelford commented on PR #14317: URL: https://github.com/apache/kafka/pull/14317#issuecomment-1706697411 > @nicktelford Thanks for the update! > > Could you also look why we did not catch this bug in `RocksDBMetricsIntegrationTest`or other metrics integration tests and add tests if needed? @cadonna Looks like that test doesn't verify the value of those metrics; it only checks the number of metrics registered under each name (i.e. that the expected metrics are registered and available, but not what they are). -- 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
[GitHub] [kafka] nicktelford commented on pull request #14317: KAFKA-13973: Fix inflated block cache metrics
nicktelford commented on PR #14317: URL: https://github.com/apache/kafka/pull/14317#issuecomment-1706624785 > I think this part of the unit tests should be affected since the called method changes: > > https://github.com/apache/kafka/blob/fd6c9f16bacf26261fee2755c1d3679b9703a8a0/streams/src/test/java/org/apache/kafka/streams/state/internals/metrics/RocksDBMetricsRecorderGaugesTest.java#L234 Woops! I must have missed those test failures. I've pushed an update to that test suite. Do you think this is sufficient or should there be more tests? -- 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
[GitHub] [kafka] nicktelford commented on pull request #14317: KAFKA-13973: Fix inflated block cache metrics
nicktelford commented on PR #14317: URL: https://github.com/apache/kafka/pull/14317#issuecomment-1706403793 > @nicktelford Could you please add new unit tests for the case of multiple column families and adapt existing unit tests to your fix? No existing tests appear to be affected by this bug/change, but I can most likely add a test that verifies the bug and fix. -- 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
