Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18569 )
Change subject: KUDU-3371 [fs] Use RocksDB to store LBM metadata ...................................................................... Patch Set 25: (4 comments) http://gerrit.cloudera.org:8080/#/c/18569/25//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18569/25//COMMIT_MSG@58 PS25, Line 58: - Adds RocksDB as a thirdparty lib I'd suggest separating this into its own patch. http://gerrit.cloudera.org:8080/#/c/18569/25/src/kudu/util/oid_generator.h File src/kudu/util/oid_generator.h: http://gerrit.cloudera.org:8080/#/c/18569/25/src/kudu/util/oid_generator.h@49 PS25, Line 49: comparation What's "comparation"? http://gerrit.cloudera.org:8080/#/c/18569/25/src/kudu/util/oid_generator.cc File src/kudu/util/oid_generator.cc: http://gerrit.cloudera.org:8080/#/c/18569/25/src/kudu/util/oid_generator.cc@66 PS25, Line 66: string ObjectIdGenerator::NextOf(const string& id) { : DCHECK(!id.empty()); : string next = id; : next[next.size() - 1] += 1; : return next; : } Separate this into its own patch and add tests for this. http://gerrit.cloudera.org:8080/#/c/18569/25/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/18569/25/thirdparty/build-definitions.sh@1177 PS25, Line 1177: -DUSE_RTTI=ON Just curious: is it possible to get away without using RTTI for the embedded RocksDB? If not, it would be great to add a comment to explain why this is needed. -- To view, visit http://gerrit.cloudera.org:8080/18569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie72f6914eb5653a9c034766c6cd3741a8340711f Gerrit-Change-Number: 18569 Gerrit-PatchSet: 25 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 17 Nov 2022 04:50:52 +0000 Gerrit-HasComments: Yes
