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 72: Code-Review+2 (4 comments) Please address one nit about using IsGTest() instead using NO_TESTS macro. Otherwise, this looks good to me! http://gerrit.cloudera.org:8080/#/c/18569/71/src/kudu/fs/CMakeLists.txt File src/kudu/fs/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/18569/71/src/kudu/fs/CMakeLists.txt@44 PS71, Line 44: kudu_util > In https://gerrit.cloudera.org/c/18569/71/src/kudu/fs/dir_manager.cc#221, I Well, NO_TESTS is rather something about the way the binaries are built, not whether some code is running in a test context. Please consider updating this and using IsGTest() utility function that's declared in test_util_prod.h http://gerrit.cloudera.org:8080/#/c/18569/71/src/kudu/fs/dir_manager.cc File src/kudu/fs/dir_manager.cc: http://gerrit.cloudera.org:8080/#/c/18569/71/src/kudu/fs/dir_manager.cc@207 PS71, Line 207: RdbDir::RdbDir(Env* env, DirMetrics* metrics, > Because it's a static member, it causes a link error if not define it here. Ah, indeed -- I missed this. Thank you for the clarification. http://gerrit.cloudera.org:8080/#/c/18569/72/src/kudu/fs/dir_manager.cc File src/kudu/fs/dir_manager.cc: http://gerrit.cloudera.org:8080/#/c/18569/72/src/kudu/fs/dir_manager.cc@220 PS72, Line 220: #if defined(NO_TESTS) Please use IsGTest() instead. You could find its usages in data_dirs.cc and a few other places. http://gerrit.cloudera.org:8080/#/c/18569/71/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/18569/71/thirdparty/build-definitions.sh@1192 PS71, Line 1192: -DWITH_LZ4=ON > I added a new option "-Dlz4_ROOT_DIR=$PREFIX" for RocksDB to find lz4 accor That's perfect, thank you! -- 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: 72 Gerrit-Owner: Yingchun Lai <laiyingc...@apache.org> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang <chinazhangyi...@163.com> Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Comment-Date: Tue, 30 Jan 2024 01:56:11 +0000 Gerrit-HasComments: Yes