[BugFix] Fix the problem of SinkIOBuffer getting stuck #40874

trueeyu · 2024-02-06T06:30:53Z

Why I'm doing:

If _cancelled is true and io queue is executing the last data chunk, the _pending_num_tasks maybe 0/1/2. So the subsequent sentinel requests cannot be executed either.

0: The io task is already pushed to queue and run, but _pending_num_tasks is not +1 now.
2: The sentinel request is pushed back to queue, and ++_pending_num_tasks.

What I'm doing:

Extract public code and put it into SinkIOBuffer
Add a SinkIOBuffer test framework
Move sentinel request before cancel check.
Rmove the dcheck of _pending_num_tasks.

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
This is a backport pr

Bugfix cherry-pick branch check:

Signed-off-by: trueeyu <lxhhust350@qq.com>

be/src/exec/pipeline/sink/sink_io_buffer.cpp

Signed-off-by: trueeyu <lxhhust350@qq.com>

stdpain · 2024-02-06T07:12:46Z

be/src/exec/pipeline/sink/sink_io_buffer.cpp

+        return;
+    }
+
+    if (_is_cancelled && !_is_finished) {


when arrive this line _is_finished is always true. it seems check !_is_finished is unnecessary

I also think it's unnecessary, but I don't understand his deep meaning here, and I'm afraid that there will be other problems if I change it, later caixiaohua will completely reconstruct it.

Signed-off-by: trueeyu <lxhhust350@qq.com>

github-actions · 2024-02-06T13:07:36Z

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2024-02-06T13:16:04Z

[BE Incremental Coverage Report]

✅ pass : 16 / 19 (84.21%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	be/src/exec/pipeline/sink/export_sink_operator.cpp	0	1	00.00%	[86]
🔵	be/src/exec/pipeline/sink/mysql_table_sink_operator.cpp	0	1	00.00%	[81]
🔵	be/src/exec/pipeline/sink/sink_io_buffer.h	3	4	75.00%	[91]
🔵	be/src/exec/pipeline/sink/file_sink_operator.cpp	1	1	100.00%	[]
🔵	be/src/exec/pipeline/sink/sink_io_buffer.cpp	12	12	100.00%	[]

github-actions · 2024-02-07T01:30:00Z

@Mergifyio backport branch-3.2

github-actions · 2024-02-07T01:30:01Z

@Mergifyio backport branch-3.1

github-actions · 2024-02-07T01:30:03Z

@Mergifyio backport branch-3.0

github-actions · 2024-02-07T01:30:05Z

@Mergifyio backport branch-2.5

mergify · 2024-02-07T01:30:33Z

backport branch-3.2

✅ Backports have been created

#40926 [BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874) has been created for branch branch-3.2 but encountered conflicts

mergify · 2024-02-07T01:30:35Z

backport branch-3.1

✅ Backports have been created

#40927 [BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874) has been created for branch branch-3.1 but encountered conflicts

mergify · 2024-02-07T01:30:36Z

backport branch-3.0

✅ Backports have been created

#40928 [BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874) has been created for branch branch-3.0 but encountered conflicts

mergify · 2024-02-07T01:30:37Z

backport branch-2.5

✅ Backports have been created

#40929 [BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874) has been created for branch branch-2.5 but encountered conflicts

Why I'm doing: If _cancelled is true and io queue is executing the last data chunk, the _pending_num_tasks maybe 0/1/2. So the subsequent sentinel requests cannot be executed either. 0: The io task is already pushed to queue and run, but _pending_num_tasks is not +1 now. 2: The sentinel request is pushed back to queue, and ++_pending_num_tasks. What I'm doing: Extract public code and put it into SinkIOBuffer Add a SinkIOBuffer test framework Move sentinel request before cancel check. Rmove the dcheck of _pending_num_tasks. Signed-off-by: trueeyu <lxhhust350@qq.com> (cherry picked from commit 50987d3) # Conflicts: # be/src/exec/pipeline/sink/export_sink_operator.cpp # be/src/exec/pipeline/sink/file_sink_operator.cpp # be/src/exec/pipeline/sink/mysql_table_sink_operator.cpp # be/test/exec/sink/sink_io_buffer_test.cpp

Why I'm doing: If _cancelled is true and io queue is executing the last data chunk, the _pending_num_tasks maybe 0/1/2. So the subsequent sentinel requests cannot be executed either. 0: The io task is already pushed to queue and run, but _pending_num_tasks is not +1 now. 2: The sentinel request is pushed back to queue, and ++_pending_num_tasks. What I'm doing: Extract public code and put it into SinkIOBuffer Add a SinkIOBuffer test framework Move sentinel request before cancel check. Rmove the dcheck of _pending_num_tasks. Signed-off-by: trueeyu <lxhhust350@qq.com> (cherry picked from commit 50987d3) # Conflicts: # be/src/exec/CMakeLists.txt

Why I'm doing: If _cancelled is true and io queue is executing the last data chunk, the _pending_num_tasks maybe 0/1/2. So the subsequent sentinel requests cannot be executed either. 0: The io task is already pushed to queue and run, but _pending_num_tasks is not +1 now. 2: The sentinel request is pushed back to queue, and ++_pending_num_tasks. What I'm doing: Extract public code and put it into SinkIOBuffer Add a SinkIOBuffer test framework Move sentinel request before cancel check. Rmove the dcheck of _pending_num_tasks. Signed-off-by: trueeyu <lxhhust350@qq.com> (cherry picked from commit 50987d3) # Conflicts: # be/src/exec/CMakeLists.txt # be/src/exec/pipeline/sink/export_sink_operator.cpp # be/src/exec/pipeline/sink/file_sink_operator.cpp # be/src/exec/pipeline/sink/mysql_table_sink_operator.cpp

…) (#40927) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>

…) (#40928) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>

…) (#40929) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>

…) (#40926) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>

Fix the problem of SinkIOBuffer getting stuck

9e2c218

Signed-off-by: trueeyu <lxhhust350@qq.com>

trueeyu requested a review from a team as a code owner February 6, 2024 06:30

mergify bot assigned trueeyu Feb 6, 2024

starrocks-cr bot reviewed Feb 6, 2024

View reviewed changes

be/src/exec/pipeline/sink/sink_io_buffer.cpp Show resolved Hide resolved

trueeyu added 3 commits February 6, 2024 14:38

Fix format

7ce6211

Signed-off-by: trueeyu <lxhhust350@qq.com>

Fix format

f59720f

Signed-off-by: trueeyu <lxhhust350@qq.com>

Fix format

d5aeea9

Signed-off-by: trueeyu <lxhhust350@qq.com>

github-actions bot added the 3.1 label Feb 6, 2024

stdpain reviewed Feb 6, 2024

View reviewed changes

kevincai previously approved these changes Feb 6, 2024

View reviewed changes

Fix

2d67dcc

Signed-off-by: trueeyu <lxhhust350@qq.com>

trueeyu dismissed kevincai’s stale review via 2d67dcc February 6, 2024 07:24

trueeyu changed the title ~~[BugFix] Fix the problem of SinkIOBuffer getting stuck~~ [WIP] [BugFix] Fix the problem of SinkIOBuffer getting stuck Feb 6, 2024

github-actions bot added the title needs [type] label Feb 6, 2024

Fix

ffba144

Signed-off-by: trueeyu <lxhhust350@qq.com>

trueeyu changed the title ~~[WIP] [BugFix] Fix the problem of SinkIOBuffer getting stuck~~ [BugFix] Fix the problem of SinkIOBuffer getting stuck Feb 6, 2024

github-actions bot added 3.2 3.0 2.5 and removed title needs [type] labels Feb 6, 2024

kevincai approved these changes Feb 6, 2024

View reviewed changes

imay approved these changes Feb 7, 2024

View reviewed changes

imay merged commit 50987d3 into StarRocks:main Feb 7, 2024
62 of 65 checks passed

github-actions bot removed 3.2 3.1 labels Feb 7, 2024

github-actions bot removed the 3.0 label Feb 7, 2024

github-actions bot removed the 2.5 label Feb 7, 2024

This was referenced Feb 7, 2024

[BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874) #40926

Merged

[BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874) #40927

Merged

mergify bot mentioned this pull request Feb 7, 2024

[BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874) #40928

Merged

18 tasks

mergify bot mentioned this pull request Feb 7, 2024

[BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874) #40929

Merged

18 tasks

wanpengfei-git pushed a commit that referenced this pull request Feb 7, 2024

[BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874…

7cdb35e

…) (#40927) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>

github-actions bot added the 3.1-merged label Feb 7, 2024

wanpengfei-git pushed a commit that referenced this pull request Feb 7, 2024

[BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874…

8e3c7db

…) (#40928) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>

github-actions bot added the 3.0-merged label Feb 7, 2024

wanpengfei-git pushed a commit that referenced this pull request Feb 15, 2024

[BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874…

c8ce029

…) (#40929) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>

github-actions bot added the 2.5-merged label Feb 15, 2024

wanpengfei-git pushed a commit that referenced this pull request Feb 16, 2024

[BugFix] Fix the problem of SinkIOBuffer getting stuck (backport #40874…

be54bc9

…) (#40926) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>

github-actions bot added 3.2-merged 3.2 3.1 3.0 2.5 labels Feb 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BugFix] Fix the problem of SinkIOBuffer getting stuck #40874

[BugFix] Fix the problem of SinkIOBuffer getting stuck #40874

trueeyu commented Feb 6, 2024 •

edited

stdpain Feb 6, 2024

trueeyu Feb 6, 2024

trueeyu Feb 6, 2024

github-actions bot commented Feb 6, 2024

github-actions bot commented Feb 6, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

mergify bot commented Feb 7, 2024 •

edited

mergify bot commented Feb 7, 2024 •

edited

mergify bot commented Feb 7, 2024 •

edited

mergify bot commented Feb 7, 2024 •

edited

[BugFix] Fix the problem of SinkIOBuffer getting stuck #40874

[BugFix] Fix the problem of SinkIOBuffer getting stuck #40874

Conversation

trueeyu commented Feb 6, 2024 • edited

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

stdpain Feb 6, 2024

Choose a reason for hiding this comment

trueeyu Feb 6, 2024

Choose a reason for hiding this comment

trueeyu Feb 6, 2024

Choose a reason for hiding this comment

github-actions bot commented Feb 6, 2024

[FE Incremental Coverage Report]

github-actions bot commented Feb 6, 2024

[BE Incremental Coverage Report]

file detail

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

mergify bot commented Feb 7, 2024 • edited

✅ Backports have been created

mergify bot commented Feb 7, 2024 • edited

✅ Backports have been created

mergify bot commented Feb 7, 2024 • edited

✅ Backports have been created

mergify bot commented Feb 7, 2024 • edited

✅ Backports have been created

trueeyu commented Feb 6, 2024 •

edited

mergify bot commented Feb 7, 2024 •

edited

mergify bot commented Feb 7, 2024 •

edited

mergify bot commented Feb 7, 2024 •

edited

mergify bot commented Feb 7, 2024 •

edited