mjsax commented on code in PR #18275:
URL: https://github.com/apache/kafka/pull/18275#discussion_r1893442075
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/metrics/RocksDBMetricsRecorder.java:
##########
@@ -462,8 +462,10 @@ public void record(final long now) {
writeStallDuration +=
valueProviders.statistics.getAndResetTickerCount(TickerType.STALL_MICROS);
bytesWrittenDuringCompaction +=
valueProviders.statistics.getAndResetTickerCount(TickerType.COMPACT_WRITE_BYTES);
bytesReadDuringCompaction +=
valueProviders.statistics.getAndResetTickerCount(TickerType.COMPACT_READ_BYTES);
- numberOfOpenFiles +=
valueProviders.statistics.getAndResetTickerCount(TickerType.NO_FILE_OPENS)
- -
valueProviders.statistics.getAndResetTickerCount(TickerType.NO_FILE_CLOSES);
+
+ // NO_FILE_CLOSES metric was removed in RocksDB 9.7.3.
+ // This now tracks the total number of file opens since the last
reset.
+ numberOfOpenFiles +=
valueProviders.statistics.getAndResetTickerCount(TickerType.NO_FILE_OPENS);
Review Comment:
As commented on the Jira ticket, changing semantics of an existing metric
does not sound right. I would rather just report `-1` surrogate for now, and
deprecate this metric with 4.0 release.
Curious to hear what others think.
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter.java:
##########
@@ -571,17 +570,6 @@ public long dbWriteBufferSize() {
return dbOptions.dbWriteBufferSize();
}
- @Override
- public Options setAccessHintOnCompactionStart(final AccessHint accessHint)
{
- dbOptions.setAccessHintOnCompactionStart(accessHint);
- return this;
- }
-
- @Override
- public AccessHint accessHintOnCompactionStart() {
- return dbOptions.accessHintOnCompactionStart();
Review Comment:
It's too bad that RockDB does not mark methods as deprecated but just
removes stuff.
This was always an issue for us (even if we have to admit, that we are
upgrade from a quite old version...). I guess we just need to bite the bullet
and make this breaking change... Seem we need to start tracking RocksDB changes
more closely and frequently and do KIPs early on to prepare for breaking
changes. (What basically implies, that we can upgrade RocksDB only in major
release if there are breaking changes...)
Or we change our "policy" for RocksDB compatibility and document it
properly, reserving the right to break compatibility via RocksDB version bumps
also in minor release.
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBGenericOptionsToDbOptionsColumnFamilyOptionsAdapter.java:
##########
@@ -842,12 +830,6 @@ public Options setSstFileManager(final SstFileManager
sstFileManager) {
return this;
}
- @Override
- public Options setLogger(final org.rocksdb.Logger logger) {
- dbOptions.setLogger(logger);
Review Comment:
If this was removed, too, can you add it to the PR description.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
