Expose GetCompactionReasonString() publicly #6489
Conversation
db/compaction/compaction.h
Outdated
| @@ -281,6 +281,8 @@ class Compaction { | |||
|
|
|||
| int GetInputBaseLevel() const; | |||
|
|
|||
| const char* GetCompactionReasonString() const; | |||
There was a problem hiding this comment.
In order to be publicly visible (i.e., usable by RocksDB users), the function needs to be declared in a header file under "include/" directory. For example take a look at how GetStringFromCompressionType() is declared:
rocksdb/include/rocksdb/convenience.h
Lines 296 to 297 in e171a21
There was a problem hiding this comment.
Sorry I probably shouldn't have provided this example here, as the declaration itself is not substantial. Meanwhile, the example function looks too similar to what our function does so it's tempting to follow its implementation pattern.
thepulkitagarwal
force-pushed
the
compaction-reason-string
branch
from
22fde0b to
e5cc030
Compare
thepulkitagarwal
changed the title
options/options_helper.cc
Outdated
| @@ -253,6 +254,25 @@ std::unordered_map<std::string, CompressionType> | |||
| {"kZSTD", kZSTD}, | |||
| {"kZSTDNotFinalCompression", kZSTDNotFinalCompression}, | |||
| {"kDisableCompressionOption", kDisableCompressionOption}}; | |||
|
|
|||
| std::unordered_map<std::string, CompactionReason> | |||
There was a problem hiding this comment.
I think we shouldn't follow GetStringFromCompressionType()'s implementation pattern - just following the declaration pattern (declaring GetStringFromCompactionReason() in a file under an "include/" directory) is enough. CompactionReason is not an option so its logic doesn't really belong here. We also don't do lookup based on string so the map isn't necessary.
Also if you want to make analogous changes for the other enums in "include/rocksdb/listener.h", that'd be helpful (e.g., TableFileCreationReason).
There was a problem hiding this comment.
I'm not sure where it should be defined. I thought of putting the definition in compaction.h, but then the other enums' definitions wouldn't fit in there. What file would you suggest for that?
There was a problem hiding this comment.
Looks like everything declared in include/rocksdb/listener.h so far is defined inline. We could either continue that pattern or create a db/listener.cc file for these definitions. Or, if you have the declarations in include/rocksdb/convenience.h, then the definitions should go in db/convenience.cc.
|
Implemented in #11778 |
For Issue #6471
Add function
GetStringFromCompactionReason(...)to get a string value from compaction reason, similar to compression type inlistener.h.