Ádám Bakai has posted comments on this change. ( http://gerrit.cloudera.org:8080/21282 )
Change subject: [CMakeLists] Make kudu_test_main static ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/21282/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21282/1//COMMIT_MSG@10 PS1, Line 10: loading chain > Please use "library search order" not "loading chain". That's the correct t I fixed the first part, and added a mention that this is a best effort fix. I saw that comment in the code that dynamic linkage isn't used in production anyway. I'm not sure it is a good argument that something is not used in production anyway when there is nothing to stop somebody to use it in production. It is true that it is statically linked on the production environment I could check. http://gerrit.cloudera.org:8080/#/c/21282/1//COMMIT_MSG@14 PS1, Line 14: This happens in ubuntu 22.04 test runs > Could you provide a set of instructions to reproduce the issue? There isn't too much change needed to get this error. On ubuntu 22.04, just run build with this: build-support/enable_devtoolset.sh ./thirdparty/build-if-necessary.sh && $KUDU_DIR/build-support/enable_devtoolset.sh \ $KUDU_DIR/thirdparty/installed/common/bin/cmake \ -DCMAKE_BUILD_TYPE=debug -G Ninja $KUDU_DIR and tests that are in kudu/gutil(for example sysinfo-test) will fail with dlopen error. But it is not 100% reproducable, sometimes it buils fine and sometimes not. It happened on my physical machine and docker and it happened on Zoltan's machine, too. This is a best effort fix from my side. It fixes the problem by making sure that statically linked resources are loaded before dynamically linked. The other solution is to somehow manipulate the library order in the executables, but I couldn't find anything about it and I couldn't even create a 100% percent working solution with manipulating the clang linking command either. http://gerrit.cloudera.org:8080/#/c/21282/1/src/kudu/util/CMakeLists.txt File src/kudu/util/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/21282/1/src/kudu/util/CMakeLists.txt@469 PS1, Line 469: add_library(kudu_test_main STATIC > The real question is: How much will this affect TSAN/ASAN build time/disk s I checked it on sysinfo-test with asan build. Without patch: File size: 8.4M Compile time(ninja sysinfo-test): 12.8 sec With patch: File size: 8.6M Compile time(ninja sysinfo-test): 12.6 sec TSAN build is not possible at the moment at all. I tested "-DKUDU_LINK=dynamic", it still seems to work and take effect: with dynamic, kudu-master executable is 75K, with static, it is 520M. I tested with only the static keyword, it sometimes still generated the dlopen error. -- To view, visit http://gerrit.cloudera.org:8080/21282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0dfeb1fa04ed91e95fd1f8d789f020dd44289fea Gerrit-Change-Number: 21282 Gerrit-PatchSet: 1 Gerrit-Owner: Ádám Bakai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Reviewer: Ádám Bakai <[email protected]> Gerrit-Comment-Date: Thu, 18 Apr 2024 13:22:23 +0000 Gerrit-HasComments: Yes
