Skip to content

Porting ALICE-specific changes#6

Merged
ktf merged 4 commits into
alisw:alice/apache-arrow-4.0.1from
aalkin:rebase-401
Jun 28, 2021
Merged

Porting ALICE-specific changes#6
ktf merged 4 commits into
alisw:alice/apache-arrow-4.0.1from
aalkin:rebase-401

Conversation

@aalkin

@aalkin aalkin commented Jun 24, 2021

Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@aalkin

aalkin commented Jun 24, 2021

Copy link
Copy Markdown
Collaborator Author

@ktf here are the changes. Let's wait for the tests to run before merging, though, maybe I have missed something.

@aalkin aalkin force-pushed the rebase-401 branch 2 times, most recently from d5699e2 to 2fdd118 Compare June 25, 2021 11:18
@pzhristov

Copy link
Copy Markdown

There is at least this issue:
/Users/phristov/alisw/sw/BUILD/8563d9b46564fe5c75f04c95c2f33b3c09dc62c8/arrow/src_tmp/cpp/src/gandiva/precompiled/arithmetic_ops.cc:265:1: error: functions that differ only in their return type cannot be overloaded
BITWISE_NOT(uint64)

@aalkin

aalkin commented Jun 25, 2021

Copy link
Copy Markdown
Collaborator Author

@pzhristov that one I already fixed, but there is gandiva-internal-test failing and I can't seem to find the reason. On top of that I cannot build arrow tests locally, neither with brew-installed google test, nor the one from alibuild. Also the latter does not seem to have an environment config.

@pzhristov

Copy link
Copy Markdown

@aalkin Thanks, at least it compiles now.

@aalkin

aalkin commented Jun 25, 2021

Copy link
Copy Markdown
Collaborator Author

@ktf @pzhristov I think I've fixed issues introduced by my code, there is still s3fs test failing, and some of the CI tests fail at installing dependencies but all that is not relevant for us. I believe this is good to go.

@pzhristov

Copy link
Copy Markdown

@aalkin Thanks, I will test it on CC7/CC8, where we had issues with 3.0.0.

@ktf ktf merged commit b745a76 into alisw:alice/apache-arrow-4.0.1 Jun 28, 2021
@ktf

ktf commented Jun 28, 2021

Copy link
Copy Markdown
Member

I've merged this so that we can try in the CI as well.

ktf pushed a commit that referenced this pull request Aug 23, 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
#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
#7  0x00007f8a5650efa0 in arrow::FutureImpl::AddCallback(arrow::internal::FnOnce<void ()>) () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
#8  0x00007f8a566e67a9 in arrow::fs::(anonymous namespace)::TreeWalker::ListObjectsV2Handler::SpawnListObjectsV2() () from /arrow/r/check/arrow.Rcheck/arrow/libs/arrow.so
#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
#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 <weston.pace@gmail.com>
Co-authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
ktf pushed a commit that referenced this pull request Aug 23, 2021
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
pzhristov pushed a commit that referenced this pull request Jul 29, 2022
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
aalkin added a commit to aalkin/arrow that referenced this pull request Oct 26, 2022
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
ktf pushed a commit that referenced this pull request Oct 26, 2022
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
aalkin added a commit to aalkin/arrow that referenced this pull request Feb 6, 2023
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
ktf pushed a commit that referenced this pull request Feb 7, 2023
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
ktf pushed a commit that referenced this pull request Feb 7, 2023
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
ktf pushed a commit that referenced this pull request Nov 14, 2023
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
aalkin added a commit that referenced this pull request Apr 25, 2024
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
aalkin added a commit that referenced this pull request Jul 31, 2024
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
ktf pushed a commit that referenced this pull request Mar 13, 2025
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
ktf pushed a commit that referenced this pull request May 27, 2025
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
aalkin added a commit that referenced this pull request Mar 11, 2026
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
aalkin added a commit that referenced this pull request Apr 22, 2026
* Additional native functions for float32 in Gandiva
* Extend bitwise operations for more int types
* Adapt to upstream trigonometric function definitions
* Prevent clash with c++ variant
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.

3 participants