[Native Execution] Build failure on MacOS when PRESTO_ENABLE_PARQUET=ON

[majetideepak]

The presto_server target build fails on MacOS with bunch of missing symbols when built with -DPRESTO_ENABLE_PARQUET=ON

bash$ PRESTO_ENABLE_PARQUET=ON make debug

.....
Undefined symbols for architecture x86_64:
  "apache::thrift::CodecConfig::__fbthrift_clear()", referenced from:
      apache::thrift::CodecConfig::~CodecConfig() in libpresto_thrift-cpp2.a(PrestoThriftAsyncClient.cpp.o)
      apache::thrift::CodecConfig::~CodecConfig() in libthriftcpp2.a(Cpp2Channel.cpp.o)
      apache::thrift::CodecConfig::~CodecConfig() in libthriftcpp2.a(RequestChannel.cpp.o)
      apache::thrift::CodecConfig::~CodecConfig() in libthriftcpp2.a(FramingHandler.cpp.o)
      apache::thrift::CodecConfig::~CodecConfig() in libthriftcpp2.a(RpcMetadataUtil.cpp.o)
  "apache::thrift::concurrency::Util::currentTimeTicks(long long)", referenced from:
      apache::thrift::concurrency::Util::currentTime() in libtransport.a(THttpParser.cpp.o)
  "apache::thrift::ContextStack::onReadData(apache::thrift::SerializedMessage const&)", referenced from:
      folly::exception_wrapper apache::thrift::detail::ac::recv_wrapped_helper<apache::thrift::BinaryProtocolReader, apache::thrift::ThriftPresult<true> >(apache::thrift::BinaryProtocolReader*, apache::thrift::ClientReceiveState&, apache::thrift::ThriftPresult<true>&) in libpresto_thrift-cpp2.a(PrestoThriftAsyncClient.cpp.o)
      folly::exception_wrapper apache::thrift::detail::ac::recv_wrapped_helper<apache::thrift::CompactProtocolReader, apache::thrift::ThriftPresult<true> >(apache::thrift::CompactProtocolReader*, apache::thrift::ClientReceiveState&, apache::thrift::ThriftPresult<true>&) in libpresto_thrift-cpp2.a(PrestoThriftAsyncClient.cpp.o)
.........

[mbasmanova]

CC: @mshang816 @amitkdutta @spershin @gggrace14

[mbasmanova]

@majetideepak Deepak, are there any particular steps to reproduce this failure? I'm able to build successfully.

[mbasmanova]

@majetideepak Deepak, does this happen only when PRESTO_ENABLE_PARQUET=ON ? I don't have this setting. I'll try to enable it and build.

[majetideepak]

@mbasmanova Yes, the build fails only when PRESTO_ENABLE_PARQUET=ON. I edited the description. Previously it was only in the title.

[majetideepak]

Some observations

1. This seems to happen only on MacOS and not on Linux.
2. We have two thrift libraries, regular thrift (for Arrow) and FBThrift. Likely a conflict between both.
3. I tried to see if we can use only FBThrift for Arrow as well. Unfortunately, this does not work since Arrow checks in regular thrift-generated Parquet Thrift CPP files.

[mshang816]

PRESTO_ENABLE_PARQUET=ON

I can take a look. So this should be reproduced like this: make -DPRESTO_ENABLE_PARQUET=ON debug ?

[majetideepak]

PRESTO_ENABLE_PARQUET=ON make debug should do.

[mshang816]

CodecConfig

it looks like I can reproduce the issue locally. I will take a look at the issue.

[uhyonc]

Hi, I have the same problem as well. Can we get an update on this ticket? Thanks!

[mbasmanova]

CC: @kgpai

[Mionsz]

@mbasmanova @majetideepak Here is the partial fix: https://github.com/prestodb/presto/pull/18213/files

You need to add also on line 158 in presto-native-execution/CMakeLists.txt (after find_library(RE2 re2)):

find_library(RE2 re2)
find_package(FBTHRIFT)
include_directories(${FBTHRIFT_INCLUDE_DIR})

You could adopt in somehow as we are waiting for legal team to finish the CLA process so it still can take some time.
P.s. Not sure if this will help with your issue :)

[majetideepak]

@Mionsz this fix did not work for me on my MacOS. Did you see the same build error in the description?

[Mionsz]

@majetideepak so now I see that we have some problems in building prestocpp - the latest version - so it is hard to tell if those are the same.
It is worth mentioning that Velox now uses ThirdpartyToolchain.cmake file for folly installation and linking. This is not reflected in PrestoCpp. There are also some issues with Presto protocol translation to PrestoCpp.

[dingqiaoyi]

Is someone working on this? Thanks in advance.

[dingqiaoyi]

@majetideepak @Mionsz

[mbasmanova]

@mshang816 Michael, is this something you could help look into?

CC: @kgpai

[Mionsz]

@dingqiaoyi @majetideepak try using this PR #18572 please give me feedback if this helps

[majetideepak]

@Mionsz the build failure happens only on MacOS. I wrote this in one of my comments above. I clarified this in the title and in the description.

[Mionsz]

Yes I am aware -- try building Thrift part with -std=c++11 -stdlib=libc++ instead of -std=c++17 -stdlib=libstdc++. It is possible that -stdlib=libc++ could help.

[markjin1990]

Yes I am aware -- try building Thrift part with -std=c++11 -stdlib=libc++ instead of -std=c++17 -stdlib=libstdc++. It is possible that -stdlib=libc++ could help.

Would you mind elaborating your proposed solution of "building Thrift part with -std=c++11 -stdlib=libc++"? Thanks!

[markjin1990]

Hi, @majetideepak Did you finally figured out a solution for this problem? Both my colleagues and I encountered this problem recently.

[sungho-park-23]

@majetideepak I am running into this issue as well, have you figured out a solution for this?

[Mionsz]

Try instead of building fbthrift running this before the build starts:

sed -i '/^sudo --preserve-env apt install/a\  *thrift* \\' presto-native-execution/scripts/setup-macos.sh

so that it will be installed

[sungho-park-23]

Try instead of building fbthrift running this before the build starts:

sed -i '/^sudo --preserve-env apt install/a\  *thrift* \\' presto-native-execution/scripts/setup-macos.sh

so that it will be installed

@Mionsz This didn't work for me, I'm still getting the same error. What exactly is this command trying to do here?

I found that the sed command itself didnt work either, so I went around it and used gsed ( looks like some differences in Ubuntu vs mac). Not sure if that caused a difference.

[majetideepak]

I am able to manually build presto_server by changing the link order of thrift/libpresto_thrift-cpp2.a /usr/local/lib/libthriftcpp2.a. I will spend more time this week on a proper fix.

[majetideepak]

I have a fix here #19021. I am able to verify locally. Can someone else verify as well?
CC: @sungho-park-23, @markjin1990
Thanks.

[markjin1990]

@majetideepak Thanks for providing the fix! This solves the problem on my side. @sungho-park-23 Please double check it.

[sungho-park-23]

I have a fix here #19021. I am able to verify locally. Can someone else verify as well? CC: @sungho-park-23, @markjin1990 Thanks.

@majetideepak It works for me! Thanks a lot.