Skip to content

Conversation

@shepmaster
Copy link
Member

No description provided.

shepmaster and others added 11 commits December 21, 2020 10:32
This function returns, potentially, a few dictionaries, and will always
return one record batch. Return these as a tuple rather than putting
them together in one vec to clarify.
This doesn't seem necessary
Tracked in ARROW-10961. There's a bug in tonic that doesn't handle
headers and trailers correctly; it has been fixed but a new version of
tonic needs to be released and used to get the fix.
@codecov-io
Copy link

codecov-io commented Dec 22, 2020

Codecov Report

Merging #5 (c3a34f7) into rust-flight-integration-tests (00512f5) will increase coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@                        Coverage Diff                        @@
##           rust-flight-integration-tests       #5      +/-   ##
=================================================================
+ Coverage                          81.91%   81.94%   +0.02%     
=================================================================
  Files                                203      203              
  Lines                              49992    49975      -17     
=================================================================
+ Hits                               40952    40953       +1     
+ Misses                              9040     9022      -18     
Impacted Files Coverage Δ
rust/arrow-flight/src/arrow.flight.protocol.rs 0.00% <ø> (ø)
...-testing/src/bin/flight-test-integration-server.rs 0.00% <0.00%> (ø)
rust/parquet/src/encodings/encoding.rs 95.43% <0.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00512f5...c3a34f7. Read the comment docs.

@carols10cents carols10cents force-pushed the rust-flight-integration-tests branch from 00512f5 to c98a22e Compare December 23, 2020 16:31
@carols10cents carols10cents force-pushed the rust-flight-integration-tests branch 3 times, most recently from 5f479dd to 116611c Compare December 28, 2020 19:47
carols10cents pushed a commit that referenced this pull request Apr 15, 2021
From a deadlocked run...

```
#0  0x00007f8a5d48dccd in __lll_lock_wait () from /lib64/libpthread.so.0
#1  0x00007f8a5d486f05 in pthread_mutex_lock () from /lib64/libpthread.so.0
#2  0x00007f8a566e7e89 in arrow::internal::FnOnce<void ()>::FnImpl<arrow::Future<Aws::Utils::Outcome<Aws::S3::Model::ListObjectsV2Result, Aws::S3::S3Error> >::Callback<arrow::fs::(anonymous namespace)::TreeWalker::ListObjectsV2Handler> >::invoke() () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
#3  0x00007f8a5650efa0 in arrow::FutureImpl::AddCallback(arrow::internal::FnOnce<void ()>) () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
#4  0x00007f8a566e67a9 in arrow::fs::(anonymous namespace)::TreeWalker::ListObjectsV2Handler::SpawnListObjectsV2() () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
#5  0x00007f8a566e723f in arrow::fs::(anonymous namespace)::TreeWalker::WalkChild(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int) () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
apache#6  0x00007f8a566e827d in arrow::internal::FnOnce<void ()>::FnImpl<arrow::Future<Aws::Utils::Outcome<Aws::S3::Model::ListObjectsV2Result, Aws::S3::S3Error> >::Callback<arrow::fs::(anonymous namespace)::TreeWalker::ListObjectsV2Handler> >::invoke() () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
apache#7  0x00007f8a5650efa0 in arrow::FutureImpl::AddCallback(arrow::internal::FnOnce<void ()>) () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
apache#8  0x00007f8a566e67a9 in arrow::fs::(anonymous namespace)::TreeWalker::ListObjectsV2Handler::SpawnListObjectsV2() () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
apache#9  0x00007f8a566e723f in arrow::fs::(anonymous namespace)::TreeWalker::WalkChild(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int) () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
apache#10 0x00007f8a566e74b1 in arrow::fs::(anonymous namespace)::TreeWalker::DoWalk() () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
```

The callback `ListObjectsV2Handler` is being called recursively and the mutex is non-reentrant thus deadlock.

To fix it I got rid of the mutex on `TreeWalker` by using `arrow::util::internal::TaskGroup` instead of manually tracking the #/status of in-flight requests.

Closes apache#9842 from westonpace/bugfix/arrow-12040

Lead-authored-by: Weston Pace <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
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.

4 participants