gitmodimo opened a new issue, #45652: URL: https://github.com/apache/arrow/issues/45652
### Describe the enhancement requested Both ConcurrentQueue and BackpressureConcurrentQueue are used only in AsofJoin and SortedMerge. Consider ConcurrentQueue as `process_` in AsofJoin: https://github.com/apache/arrow/blob/0fbf9823542233c5f32c26534c34cc97ce3f0be2/cpp/src/arrow/acero/asof_join_node.cc#L1104-L1114 and as `process_queue` in SortedMerge: https://github.com/apache/arrow/blob/0fbf9823542233c5f32c26534c34cc97ce3f0be2/cpp/src/arrow/acero/sorted_merge_node.cc#L586-L597 In both cases the code relies on Pop() to block on empty queue. It can be confirmed with current tests when you remove cond_.wait from: https://github.com/apache/arrow/blob/0fbf9823542233c5f32c26534c34cc97ce3f0be2/cpp/src/arrow/acero/concurrent_queue_internal.h#L36-L40 This all seems fine and all if we assert Pop is a blocking. But then derived class BackpressureConcurrentQueue break this assertion with non blocking implementation of Pop. Even though in both cases it is used correctly (ie. tested for emptiness before pop) it makes the code messy. I think the API should be made a little more explicit about blocking/non-blocking behaviour and less confusing for further use and ARROW_EXPORT it. Side note. Confusing API could be the reason for this: https://github.com/apache/arrow/blob/0fbf9823542233c5f32c26534c34cc97ce3f0be2/cpp/src/arrow/acero/asof_join_node.cc#L641-L646 In this context queue_.TryPop() always succeeds and `have_active_batch` is always false. And that actually saves us from UB by queue_.Front() in empty after pop queue. ### Component(s) C++ -- 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: issues-unsubscr...@arrow.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
