Skip to content

Conversation

@aucahuasi
Copy link
Contributor

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

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

@github-actions
Copy link

@pitrou
Copy link
Member

pitrou commented Oct 26, 2022

Why only in some tests?

@lidavidm
Copy link
Member

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

@westonpace
Copy link
Member

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

@aucahuasi aucahuasi force-pushed the ARROW-18070-valgrind-leaks-substrait-tests branch from 17a3ead to 790cebd Compare October 26, 2022 21:45
…m leaks

format

invoke google::protobuf::ShutdownProtobufLibrary for all substrait tests
@aucahuasi aucahuasi force-pushed the ARROW-18070-valgrind-leaks-substrait-tests branch from 790cebd to 69d2477 Compare October 26, 2022 21:49
@aucahuasi
Copy link
Contributor Author

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

@aucahuasi aucahuasi changed the title ARROW-18070: [C++] Invoke google::protobuf::ShutdownProtobufLibrary in some tests ARROW-18070: [C++] Invoke google::protobuf::ShutdownProtobufLibrary for substrait tests Oct 26, 2022
@pitrou
Copy link
Member

pitrou commented Oct 27, 2022

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

@aucahuasi
Copy link
Contributor Author

@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)

@pitrou pitrou requested a review from lidavidm October 27, 2022 16:28
@lidavidm
Copy link
Member

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

@github-actions
Copy link

Revision: 2ef6691

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

Task Status
test-conda-cpp-valgrind Azure

@lidavidm
Copy link
Member

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

@lidavidm
Copy link
Member

LGTM once we have the doc comment

@lidavidm lidavidm merged commit 2898060 into apache:master Oct 28, 2022
@ursabot
Copy link

ursabot commented Oct 30, 2022

Benchmark runs are scheduled for baseline = 4e7d91c and contender = 2898060. 2898060 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.56% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.11% ⬆️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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants