Update HistogramTypes in the Java API #2429
Conversation
|
@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
Not sure why appveyor build didn't get kicked off; i'll rebase to kick off the windows build. --> Didn't work. |
sagar0
force-pushed
the
update-histograms-in-java
branch
from
99a76c0
to
1007b60
Compare
|
@sagar0 updated the pull request - view changes - changes since last import |
| TABLE_SYNC_MICROS(4), | ||
| COMPACTION_OUTFILE_SYNC_MICROS(5), | ||
| WAL_FILE_SYNC_MICROS(6), | ||
| MANIFEST_FILE_SYNC_MICROS(7), |
There was a problem hiding this comment.
this has been changed from 6 to 7, do we need to change the counter part in C++ code as well?
There was a problem hiding this comment.
No, C++ enums are fine.
The problem is the Java histogramTypes enums are not currently in sync with the C++ ones. The C++ enums are here: https:/facebook/rocksdb/blob/master/include/rocksdb/statistics.h#L339-L341 , for reference. A few years back SUBCOMPACTION_SETUP_TIME was added in between instead of at the end in C++, due to which Java enums became out of sync. (I also mentioned this in the PR description, just so that people are aware).
I believe the histograms in Java are getting incorrect data due to this (I cannot confirm though). So, I updated the java enums to be now in sync with the C++ enums.
|
The test failures are unrelated to this diff. |
|
@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: This diff syncs the Histogram Types in the Java API with the ones in C++ API (`statistics.h`), and brings it up-to-date. I also found that the enum ordering between Java and C++ has gotten out-of-sync, a few years back, with the addition of `SUBCOMPACTION_SETUP_TIME`. So updated the order as well. `READ_NUM_MERGE_OPERANDS` added in #2373 is needed for Cassandra-on-RocksDB work. Closes #2429 Differential Revision: D5215623 Pulled By: sagar0 fbshipit-source-id: bd136698c48197e53693275eb52acc9198ee5a4e
This diff syncs the Histogram Types in the Java API with the ones in C++ API (
statistics.h), and brings it up-to-date.I also found that the enum ordering between Java and C++ has gotten out-of-sync, a few years back, with the addition of
SUBCOMPACTION_SETUP_TIME. So updated the order as well.READ_NUM_MERGE_OPERANDSadded in #2373 is needed for Cassandra-on-RocksDB work.Test Plan:
I couldn't find any specific unit tests in Java to test this code. But, nevertheless,
make jtestexecutes fine.