You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/02/02 23:44:35 UTC

[GitHub] [arrow] kou opened a new pull request, #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

kou opened a new pull request, #34014:
URL: https://github.com/apache/arrow/pull/34014

   ### Rationale for this change
   
   We want to reduce CI time.
   
   ### What changes are included in this PR?
   
   This disables Flight SQL to use system gRPC and ProtoBuf.
   We can reduce build time by avoiding bundled gRPC and ProtoBuf.
   
   ### Are these changes tested?
   
   Yes.
   
   ### Are there any user-facing changes?
   
   No.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] westonpace commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1414597743

   I ran this locally and does prevent building protobuf.  We still might also want to turn off GCS (although that can be a separate PR)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1414725466

   > We still might also want to turn off GCS (although that can be a separate PR)
   
   OK. Let's use separated PR.
   
   > Maybe flight (and not just flight SQL) require newer protobuf?
   
   Oh... Can someone suggest a fix for this? If we need newer ProtoBuf, we need to disable Flight too for now.
   
   https://github.com/apache/arrow/actions/runs/4079672860/jobs/7031285408#step:6:2775
   
   ```text
   /usr/include/grpcpp/impl/codegen/client_context_impl.h:477:13: runtime error: load of value 48, which is not a valid value for type 'bool'
       #0 0x7fe9f2a2323b in grpc_impl::ClientContext::initial_metadata_flags() const /usr/include/grpcpp/impl/codegen/client_context_impl.h:477:13
       #1 0x7fe9f2a93c94 in grpc::internal::BlockingUnaryCallImpl<arrow::flight::protocol::FlightDescriptor, arrow::flight::protocol::FlightInfo>::BlockingUnaryCallImpl(grpc::ChannelInterface*, grpc::internal::RpcMethod const&, grpc_impl::ClientContext*, arrow::flight::protocol::FlightDescriptor const&, arrow::flight::protocol::FlightInfo*) /usr/include/grpcpp/impl/codegen/client_unary_call.h:65:38
       #2 0x7fe9f2a3aa47 in grpc::Status grpc::internal::BlockingUnaryCall<arrow::flight::protocol::FlightDescriptor, arrow::flight::protocol::FlightInfo>(grpc::ChannelInterface*, grpc::internal::RpcMethod const&, grpc_impl::ClientContext*, arrow::flight::protocol::FlightDescriptor const&, arrow::flight::protocol::FlightInfo*) /usr/include/grpcpp/impl/codegen/client_unary_call.h:41:10
       #3 0x7fe9f2a3a80a in arrow::flight::protocol::FlightService::Stub::GetFlightInfo(grpc_impl::ClientContext*, arrow::flight::protocol::FlightDescriptor const&, arrow::flight::protocol::FlightInfo*) /build/cpp/src/arrow/flight/Flight.grpc.pb.cc:89:10
       #4 0x7fe9f28ccfd7 in arrow::flight::transport::grpc::(anonymous namespace)::GrpcClientImpl::GetFlightInfo(arrow::flight::FlightCallOptions const&, arrow::flight::FlightDescriptor const&, std::unique_ptr<arrow::flight::FlightInfo, std::default_delete<arrow::flight::FlightInfo> >*) /arrow/cpp/src/arrow/flight/transport/grpc/grpc_client.cc:829:16
       #5 0x7fe9f25fb029 in arrow::flight::FlightClient::GetFlightInfo(arrow::flight::FlightCallOptions const&, arrow::flight::FlightDescriptor const&) /arrow/cpp/src/arrow/flight/client.cc:589:3
       #6 0x5631ef06e3a0 in arrow::flight::FlightClient::GetFlightInfo(arrow::flight::FlightDescriptor const&) /arrow/cpp/src/arrow/flight/client.h:275:12
       #7 0x7fe9f56047fb in arrow::flight::ConnectivityTest::TestBrokenConnection() /arrow/cpp/src/arrow/flight/test_definitions.cc:111:3
       #8 0x5631ef041e60 in arrow::flight::GrpcConnectivityTest_BrokenConnection_Test::TestBody() /arrow/cpp/src/arrow/flight/flight_test.cc:104:1
       #9 0x7fe9f33ae5aa in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:2607:10
       #10 0x7fe9f3392c19 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:2643:14
       #11 0x7fe9f336ca82 in testing::Test::Run() /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:2682:5
       #12 0x7fe9f336d7e8 in testing::TestInfo::Run() /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:2861:11
       #13 0x7fe9f336e003 in testing::TestSuite::Run() /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:3015:28
       #14 0x7fe9f337e981 in testing::internal::UnitTestImpl::RunAllTests() /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:5855:44
       #15 0x7fe9f33b15aa in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:2607:10
       #16 0x7fe9f3395419 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:2643:14
       #17 0x7fe9f337e4ea in testing::UnitTest::Run() /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest.cc:5438:10
       #18 0x7fe9f33ea210 in RUN_ALL_TESTS() /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/include/gtest/gtest.h:2490:46
       #19 0x7fe9f33ea1ec in main /build/cpp/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest_main.cc:52:10
       #0 0x7fe9d5cf4d8f in
       #21 0x7fe9d5cf4e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
       #22 0x5631eef80544 in _start (/build/cpp/debug/arrow-flight-test+0x3e9544) (BuildId: 7abc0a59c2ea24de67ae219bc9f76a90e6a94ab2)
   
   SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/include/grpcpp/impl/codegen/client_context_impl.h:477:13 in
   /build/cpp/src/arrow/flight
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1414518743

   * Closes: #33920


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1416765236

   Good catch!
   I've added `-DGRPC_ASAN_SUPPRESSED`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] js8544 commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "js8544 (via GitHub)" <gi...@apache.org>.
js8544 commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1416618143

   I can also reproduce this locally. Here's what I can find out with gdb:
   
   1. The UB happens because `rpc.context.initial_metadata_corked_` is not initialized.
   2. The constructor `ClientContext::ClientContext()` is called, but only a few member variables are initialized, `initial_metadata_corked_` is not one of them.
   3.  This only happens when UBSAN and ASAN are on. In a regular build `rpc.context` is properly initialized.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] westonpace commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1419232557

   @js8544 great catch indeed!  Very impressive investigating.  Thanks for looking at it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] westonpace commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1416547018

   I spent quite some time today trying to debug this with no luck.  It is a very consistent failure in this configuration.  I can reproduce with docker.
   
   From what I can tell it is including and linking the correct versions of protobuf and grpc.
   
   gdb isn't terribly useful (grpc's symbols are stripped and so I can't inspect the rpc.context object).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1414518757

   :warning: GitHub issue #33920 **has been automatically assigned in GitHub** to PR creator.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] js8544 commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "js8544 (via GitHub)" <gi...@apache.org>.
js8544 commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1416752797

   I found the root cause. GRPC's [`gpr_mu`](https://github.com/grpc/grpc/blob/master/include/grpc/support/sync_posix.h#L28) mutex has different structures depending on whether UBSAN is on. So we have to compile GRPC by ourselves if we want to enable UBSAN.
   cc @kou @westonpace 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1416923501

   +1
   
   Passed in 34min: https://github.com/apache/arrow/actions/runs/4092011740/jobs/7056458825


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou merged pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #34014:
URL: https://github.com/apache/arrow/pull/34014


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] ursabot commented on pull request #34014: GH-33920: [C++][CI] Disable Flight SQL in sanitizer job

Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #34014:
URL: https://github.com/apache/arrow/pull/34014#issuecomment-1417046106

   Benchmark runs are scheduled for baseline = ecb1f2e31d73b0ccc7b324d5e25eb015f6524bc6 and contender = 8233ed4545212fb086d60715c80257e755b9a192. 8233ed4545212fb086d60715c80257e755b9a192 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/5df1d3d29e4c475496b10d5ab78537f5...c1b20234f80c4091b6aad58eafa03d80/)
   [Failed :arrow_down:0.24% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/5568cda0011546baa698871638287a55...bba0dbce7991464e9139fc518aeee999/)
   [Finished :arrow_down:0.77% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/560dd0e7f2f241af92d1fa83c7ce9e1f...d1f712c7992a42a5b7344aceb7103a6c/)
   [Finished :arrow_down:0.35% :arrow_up:0.13%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/0966424ff0fd4c1bada84ee8c5bc5c21...e27d51902dc5429983741d57f6486ac0/)
   Buildkite builds:
   [Finished] [`8233ed45` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2327)
   [Failed] [`8233ed45` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2355)
   [Finished] [`8233ed45` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2325)
   [Finished] [`8233ed45` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2346)
   [Finished] [`ecb1f2e3` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2326)
   [Failed] [`ecb1f2e3` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2354)
   [Finished] [`ecb1f2e3` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2324)
   [Finished] [`ecb1f2e3` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2345)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org