-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
[BugFix] Fix the has_output check bug of scan operator #42994
Conversation
Signed-off-by: trueeyu <lxhhust350@qq.com>
static_cast<void>(_ctx->set_finished()); | ||
TEST_SYNC_POINT("OlapScnPrepareOperator::pull_chunk::after_set_finished"); | ||
_ctx->set_prepare_finished(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defer set_prepare_finished will be OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
[FE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]✅ pass : 9 / 10 (90.00%) file detail
|
@Mergifyio backport branch-3.2 |
@Mergifyio backport branch-3.1 |
@Mergifyio backport branch-3.0 |
✅ Backports have been created
|
@Mergifyio backport branch-2.5 |
✅ Backports have been created
|
✅ Backports have been created
|
✅ Backports have been created
|
Signed-off-by: trueeyu <lxhhust350@qq.com> (cherry picked from commit d9d9c70) # Conflicts: # be/src/exec/pipeline/scan/olap_scan_prepare_operator.cpp
Signed-off-by: trueeyu <lxhhust350@qq.com> (cherry picked from commit d9d9c70) # Conflicts: # be/src/exec/pipeline/scan/olap_scan_prepare_operator.cpp
Signed-off-by: trueeyu <lxhhust350@qq.com> (cherry picked from commit d9d9c70) # Conflicts: # be/src/exec/pipeline/scan/olap_scan_prepare_operator.cpp # be/src/testutil/sync_point.h
Signed-off-by: trueeyu <lxhhust350@qq.com> (cherry picked from commit d9d9c70) # Conflicts: # be/src/exec/pipeline/scan/olap_scan_prepare_operator.cpp # be/src/testutil/sync_point.h # be/test/CMakeLists.txt
…) (#43003) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>
…) (#43004) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>
…) (#43005) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>
…) (#43006) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>
…Rocks#42994) (StarRocks#43006) Signed-off-by: trueeyu <lxhhust350@qq.com> Co-authored-by: trueeyu <lxhhust350@qq.com>
Why I'm doing:
What we expect is that
ScanOperator::pull_chunk
can be executed only afterScanPrepareOperator::pull_chunk
is executed finished and return success.If
ScanPrepareOperator::pull_chunk
returns EOF,ScanOperator::pull_chunk
should not be executed.But when in function
OlapScanPrepareOperator::pull_chunk
, if_ctx->set_prepare_finished
is executed finished and before _ctx->set_finished() is executed,will return true, causing
OlapScanOperator::pull_chunk
will be executed, causing some unexpected crashes.There are dependencies between operators. The current scheduling does not consider this dependency, which is not a good design.
But the current fix does not change the implementation of this pipeline engine.
What I'm doing:
If prepare return eof, we will first
set_finished
and thenset_prepare_finished
.What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: