Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++] Acero will hang if StopProducing is called while backpressure is applied on the source node #35837

Closed
westonpace opened this issue May 31, 2023 · 1 comment · Fixed by #35902
Closed

[C++] Acero will hang if StopProducing is called while backpressure is applied on the source node #35837

westonpace opened this issue May 31, 2023 · 1 comment · Fixed by #35902
Assignees
@rtpsw
Labels
Component: C++ Type: bug
Milestone
13.0.0

Comments

@westonpace
Copy link
Member

Describe the bug, including details regarding any error messages, version, and platform.

The source node creates an external task called SourceNode::DatasetScan which is managed outside the exec plan. This task is actually an arrow::Loop call which is iterating the source async generator. That loop can be paused and resumed to apply backpressure.

When StopProducing arrives the source node sets stop_requested_=true. However, stop_requested_ is not processed until the next iteration through the loop. If the plan is paused then the loop is not iterating and nothing notices that stop_requested_ changed and the external task never completes and the plan then never completes.

Component(s)

C++
@rtpsw
Copy link
Contributor

rtpsw commented Jun 4, 2023

take

rtpsw added a commit to rtpsw/arrow that referenced this issue Jun 4, 2023
@rtpsw
apacheGH-35837: [C++] Acero will hang if StopProducing is called whil…
e29a922 
…e backpressure is applied on the source node
@github-actions github-actions bot mentioned this issue Jun 4, 2023
Merged
rtpsw added a commit to rtpsw/arrow that referenced this issue Jun 4, 2023
@rtpsw
apacheGH-35837: [C++] Acero will hang if StopProducing is called whil…
30a1224 
…e backpressure is applied on the source node
rtpsw added a commit to rtpsw/arrow that referenced this issue Jun 4, 2023
@rtpsw
apacheGH-35837: [C++] Acero will hang if StopProducing is called whil…
148d935 
…e backpressure is applied on the source node
rtpsw added a commit to rtpsw/arrow that referenced this issue Jun 4, 2023
@rtpsw
apacheGH-35837: [C++] Acero will hang if StopProducing is called whil…
1d978c0 
…e backpressure is applied on the source node
rtpsw added a commit to rtpsw/arrow that referenced this issue Jun 4, 2023
@rtpsw
apacheGH-35837: [C++] Acero will hang if StopProducing is called whil…
d497794 
…e backpressure is applied on the source node
westonpace pushed a commit to westonpace/arrow that referenced this issue Jun 15, 2023
@rtpsw @westonpace
apacheGH-35837: [C++] Acero will hang if StopProducing is called whil…
a00a56a 
…e backpressure is applied on the source node
icexelloss pushed a commit that referenced this issue Jun 16, 2023
@rtpsw
GH-35837: [C++] Acero will hang if StopProducing is called while back…
e39386b 
…pressure is applied on the source node (#35902)

### What changes are included in this PR?

The stopping logic for `SourceNode` now ensures the node is first resumed.

### Are these changes tested?

Yes. A new `source_node_test` tester is added for this.

### Are there any user-facing changes?

No.

**This PR contains a "Critical Fix".**

* Closes: #35837 

Authored-by: Yaron Gvili <rtpsw@hotmail.com>
Signed-off-by: Li Jin <ice.xelloss@gmail.com>
@icexelloss icexelloss added this to the 13.0.0 milestone Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rtpsw rtpsw

Labels
Component: C++ Type: bug
Projects
None yet
Milestone
13.0.0
3 participants
@icexelloss @westonpace @rtpsw