arrow icon indicating copy to clipboard operation
arrow copied to clipboard

ARROW-18070: [C++] Invoke google::protobuf::ShutdownProtobufLibrary for substrait tests

Open aucahuasi opened this issue 1 year ago • 11 comments

Invoke google::protobuf::ShutdownProtobufLibrary to avoid memory issues detected by valgrind.

Jira ticket: https://issues.apache.org/jira/browse/ARROW-18070

aucahuasi avatar Oct 25 '22 22:10 aucahuasi

https://issues.apache.org/jira/browse/ARROW-18070

github-actions[bot] avatar Oct 25 '22 23:10 github-actions[bot]

Why only in some tests?

pitrou avatar Oct 26 '22 10:10 pitrou

From the docs, it sounds like we should find a way to just do this globally, for all tests? https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.message_lite#ShutdownProtobufLibrary.details

lidavidm avatar Oct 26 '22 12:10 lidavidm

I agree with @pitrou and @lidavidm . We will want this on all tests that use protobuf. Perhaps something like the way we setup and teardown S3 here: https://github.com/apache/arrow/blob/master/cpp/src/arrow/filesystem/s3fs_test.cc#L86

westonpace avatar Oct 26 '22 14:10 westonpace

Thank you all for the review, I sent some changes!

aucahuasi avatar Oct 26 '22 21:10 aucahuasi

@aucahuasi Did you validate that it fixes the Valgrind issues?

pitrou avatar Oct 27 '22 12:10 pitrou

@aucahuasi Did you validate that it fixes the Valgrind issues?

Yes, that was the first thing I did! 👍 Here is the output that I ran yesterday using the PR:

valgrind --suppressions=/devenv/arrow/cpp/valgrind.supp --tool=memcheck --gen-suppressions=all --num-callers=500 --leak-check=full --leak-check-heuristics=stdstring --error-exitcode=1 --show-leak-kinds=all debug/arrow-substrait-substrait-test
==2575417== Memcheck, a memory error detector
==2575417== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2575417== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==2575417== Command: debug/arrow-substrait-substrait-test
==2575417==
Running main() from /devenv/arrow/cpp/build/googletest_ep-prefix/src/googletest_ep/googletest/src/gtest_main.cc
[==========] Running 63 tests from 4 test suites.
[----------] Global test environment set-up.
[----------] 5 tests from ExtensionIdRegistryTest
[ RUN      ] ExtensionIdRegistryTest.GetSupportedSubstraitFunctions
[       OK ] ExtensionIdRegistryTest.GetSupportedSubstraitFunctions (70 ms)
[ RUN      ] ExtensionIdRegistryTest.RegisterTempTypes
[       OK ] ExtensionIdRegistryTest.RegisterTempTypes (23 ms)
[ RUN      ] ExtensionIdRegistryTest.RegisterTempFunctions
[       OK ] ExtensionIdRegistryTest.RegisterTempFunctions (16 ms)
[ RUN      ] ExtensionIdRegistryTest.RegisterNestedTypes
[       OK ] ExtensionIdRegistryTest.RegisterNestedTypes (18 ms)
[ RUN      ] ExtensionIdRegistryTest.RegisterNestedFunctions
[       OK ] ExtensionIdRegistryTest.RegisterNestedFunctions (17 ms)
[----------] 5 tests from ExtensionIdRegistryTest (164 ms total)

[----------] 3 tests from FunctionMapping
[ RUN      ] FunctionMapping.ValidCases
[       OK ] FunctionMapping.ValidCases (7198 ms)
[ RUN      ] FunctionMapping.ErrorCases
[       OK ] FunctionMapping.ErrorCases (116 ms)
[ RUN      ] FunctionMapping.AggregateCases
[       OK ] FunctionMapping.AggregateCases (1603 ms)
[----------] 3 tests from FunctionMapping (8919 ms total)

[----------] 47 tests from Substrait
[ RUN      ] Substrait.SupportedTypes
[       OK ] Substrait.SupportedTypes (541 ms)
[ RUN      ] Substrait.SupportedExtensionTypes
[       OK ] Substrait.SupportedExtensionTypes (72 ms)
[ RUN      ] Substrait.NamedStruct
[       OK ] Substrait.NamedStruct (121 ms)
[ RUN      ] Substrait.NoEquivalentArrowType
[       OK ] Substrait.NoEquivalentArrowType (27 ms)
[ RUN      ] Substrait.NoEquivalentSubstraitType
[       OK ] Substrait.NoEquivalentSubstraitType (73 ms)
[ RUN      ] Substrait.SupportedLiterals
[       OK ] Substrait.SupportedLiterals (976 ms)
[ RUN      ] Substrait.CannotDeserializeLiteral
[       OK ] Substrait.CannotDeserializeLiteral (23 ms)
[ RUN      ] Substrait.FieldRefRoundTrip
[       OK ] Substrait.FieldRefRoundTrip (164 ms)
[ RUN      ] Substrait.RecursiveFieldRef
[       OK ] Substrait.RecursiveFieldRef (27 ms)
[ RUN      ] Substrait.FieldRefsInExpressions
[       OK ] Substrait.FieldRefsInExpressions (90 ms)
[ RUN      ] Substrait.CallSpecialCaseRoundTrip
[       OK ] Substrait.CallSpecialCaseRoundTrip (197 ms)
[ RUN      ] Substrait.CallExtensionFunction
[       OK ] Substrait.CallExtensionFunction (31 ms)
[ RUN      ] Substrait.ReadRel
[       OK ] Substrait.ReadRel (380 ms)
[ RUN      ] Substrait.RelWithHint
[       OK ] Substrait.RelWithHint (26 ms)
[ RUN      ] Substrait.ExtensionSetFromPlan
[       OK ] Substrait.ExtensionSetFromPlan (52 ms)
[ RUN      ] Substrait.ExtensionSetFromPlanMissingFunc
[       OK ] Substrait.ExtensionSetFromPlanMissingFunc (12 ms)
[ RUN      ] Substrait.ExtensionSetFromPlanExhaustedFactory
[       OK ] Substrait.ExtensionSetFromPlanExhaustedFactory (27 ms)
[ RUN      ] Substrait.ExtensionSetFromPlanRegisterFunc
[       OK ] Substrait.ExtensionSetFromPlanRegisterFunc (13 ms)
[ RUN      ] Substrait.DeserializeWithConsumerFactory
[       OK ] Substrait.DeserializeWithConsumerFactory (2175 ms)
[ RUN      ] Substrait.DeserializeSinglePlanWithConsumerFactory
[       OK ] Substrait.DeserializeSinglePlanWithConsumerFactory (62 ms)
[ RUN      ] Substrait.DeserializeWithWriteOptionsFactory
[       OK ] Substrait.DeserializeWithWriteOptionsFactory (810 ms)
[ RUN      ] Substrait.GetRecordBatchReader
[       OK ] Substrait.GetRecordBatchReader (459 ms)
[ RUN      ] Substrait.InvalidPlan
[       OK ] Substrait.InvalidPlan (13 ms)
[ RUN      ] Substrait.JoinPlanBasic
[       OK ] Substrait.JoinPlanBasic (88 ms)
[ RUN      ] Substrait.JoinPlanInvalidKeyCmp
[       OK ] Substrait.JoinPlanInvalidKeyCmp (32 ms)
[ RUN      ] Substrait.JoinPlanInvalidExpression
[       OK ] Substrait.JoinPlanInvalidExpression (19 ms)
[ RUN      ] Substrait.JoinPlanInvalidKeys
[       OK ] Substrait.JoinPlanInvalidKeys (11 ms)
[ RUN      ] Substrait.AggregateBasic
[       OK ] Substrait.AggregateBasic (19 ms)
[ RUN      ] Substrait.AggregateInvalidRel
[       OK ] Substrait.AggregateInvalidRel (6 ms)
[ RUN      ] Substrait.AggregateInvalidFunction
[       OK ] Substrait.AggregateInvalidFunction (11 ms)
[ RUN      ] Substrait.AggregateInvalidAggFuncArgs
[       OK ] Substrait.AggregateInvalidAggFuncArgs (14 ms)
[ RUN      ] Substrait.AggregateWithFilter
[       OK ] Substrait.AggregateWithFilter (16 ms)
[ RUN      ] Substrait.AggregateBadPhase
[       OK ] Substrait.AggregateBadPhase (15 ms)
[ RUN      ] Substrait.BasicPlanRoundTripping
[       OK ] Substrait.BasicPlanRoundTripping (336 ms)
[ RUN      ] Substrait.BasicPlanRoundTrippingEndToEnd
[       OK ] Substrait.BasicPlanRoundTrippingEndToEnd (883 ms)
[ RUN      ] Substrait.ProjectRel
[       OK ] Substrait.ProjectRel (87 ms)
[ RUN      ] Substrait.ProjectRelOnFunctionWithEmit
[       OK ] Substrait.ProjectRelOnFunctionWithEmit (71 ms)
[ RUN      ] Substrait.ReadRelWithEmit
[       OK ] Substrait.ReadRelWithEmit (41 ms)
[ RUN      ] Substrait.FilterRelWithEmit
[       OK ] Substrait.FilterRelWithEmit (58 ms)
[ RUN      ] Substrait.JoinRelEndToEnd
[       OK ] Substrait.JoinRelEndToEnd (704 ms)
[ RUN      ] Substrait.JoinRelWithEmit
[       OK ] Substrait.JoinRelWithEmit (164 ms)
[ RUN      ] Substrait.AggregateRel
[       OK ] Substrait.AggregateRel (77 ms)
[ RUN      ] Substrait.AggregateRelEmit
[       OK ] Substrait.AggregateRelEmit (46 ms)
[ RUN      ] Substrait.IsthmusPlan
[       OK ] Substrait.IsthmusPlan (63 ms)
[ RUN      ] Substrait.ProjectWithMultiFieldExpressions
[       OK ] Substrait.ProjectWithMultiFieldExpressions (50 ms)
[ RUN      ] Substrait.NestedProjectWithMultiFieldExpressions
[       OK ] Substrait.NestedProjectWithMultiFieldExpressions (88 ms)
[ RUN      ] Substrait.NestedEmitProjectWithMultiFieldExpressions
[       OK ] Substrait.NestedEmitProjectWithMultiFieldExpressions (43 ms)
[----------] 47 tests from Substrait (9363 ms total)

[----------] 8 tests from Substrait/ExtensionIdRegistryTest
[ RUN      ] Substrait/ExtensionIdRegistryTest.GetTypes/0
[       OK ] Substrait/ExtensionIdRegistryTest.GetTypes/0 (8 ms)
[ RUN      ] Substrait/ExtensionIdRegistryTest.GetTypes/1
[       OK ] Substrait/ExtensionIdRegistryTest.GetTypes/1 (0 ms)
[ RUN      ] Substrait/ExtensionIdRegistryTest.ReregisterTypes/0
[       OK ] Substrait/ExtensionIdRegistryTest.ReregisterTypes/0 (4 ms)
[ RUN      ] Substrait/ExtensionIdRegistryTest.ReregisterTypes/1
[       OK ] Substrait/ExtensionIdRegistryTest.ReregisterTypes/1 (2 ms)
[ RUN      ] Substrait/ExtensionIdRegistryTest.GetFunctions/0
[       OK ] Substrait/ExtensionIdRegistryTest.GetFunctions/0 (10 ms)
[ RUN      ] Substrait/ExtensionIdRegistryTest.GetFunctions/1
[       OK ] Substrait/ExtensionIdRegistryTest.GetFunctions/1 (0 ms)
[ RUN      ] Substrait/ExtensionIdRegistryTest.ReregisterFunctions/0
[       OK ] Substrait/ExtensionIdRegistryTest.ReregisterFunctions/0 (3 ms)
[ RUN      ] Substrait/ExtensionIdRegistryTest.ReregisterFunctions/1
[       OK ] Substrait/ExtensionIdRegistryTest.ReregisterFunctions/1 (0 ms)
[----------] 8 tests from Substrait/ExtensionIdRegistryTest (32 ms total)

[----------] Global test environment tear-down
[==========] 63 tests from 4 test suites ran. (18527 ms total)
[  PASSED  ] 63 tests.
==2575417==
==2575417== HEAP SUMMARY:
==2575417==     in use at exit: 1,344 bytes in 4 blocks
==2575417==   total heap usage: 169,677 allocs, 169,673 frees, 17,979,749 bytes allocated
==2575417==
==2575417== LEAK SUMMARY:
==2575417==    definitely lost: 0 bytes in 0 blocks
==2575417==    indirectly lost: 0 bytes in 0 blocks
==2575417==      possibly lost: 0 bytes in 0 blocks
==2575417==    still reachable: 0 bytes in 0 blocks
==2575417==         suppressed: 1,344 bytes in 4 blocks
==2575417==
==2575417== For lists of detected and suppressed errors, rerun with: -s
==2575417== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)

aucahuasi avatar Oct 27 '22 14:10 aucahuasi

@github-actions crossbow submit test-conda-cpp-valgrind

lidavidm avatar Oct 28 '22 11:10 lidavidm

Revision: 2ef66913dc815ea812c25810f2e5259891b4abb9

Submitted crossbow builds: ursacomputing/crossbow @ actions-342c4b87fd

Task Status
test-conda-cpp-valgrind Azure

github-actions[bot] avatar Oct 28 '22 12:10 github-actions[bot]

Filed https://issues.apache.org/jira/browse/ARROW-18191 since it seems we have a separate valgrind failure now

lidavidm avatar Oct 28 '22 15:10 lidavidm

LGTM once we have the doc comment

lidavidm avatar Oct 28 '22 15:10 lidavidm

Benchmark runs are scheduled for baseline = 4e7d91cd7ad42c10195ef465f5b6f1daf7b72f05 and contender = 2898060c6417e76ebbf96675f542a4c6b71ea57b. 2898060c6417e76ebbf96675f542a4c6b71ea57b 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 [Failed :arrow_down:0.56% :arrow_up:0.0%] test-mac-arm [Finished :arrow_down:0.0% :arrow_up:0.0%] ursa-i9-9960x [Finished :arrow_down:0.11% :arrow_up:0.0%] ursa-thinkcentre-m75q Buildkite builds: [Finished] 2898060c ec2-t3-xlarge-us-east-2 [Failed] 2898060c test-mac-arm [Finished] 2898060c ursa-i9-9960x [Finished] 2898060c ursa-thinkcentre-m75q [Finished] 4e7d91cd ec2-t3-xlarge-us-east-2 [Failed] 4e7d91cd test-mac-arm [Finished] 4e7d91cd ursa-i9-9960x [Finished] 4e7d91cd ursa-thinkcentre-m75q 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

ursabot avatar Oct 30 '22 07:10 ursabot