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.

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] Acquire rowsets at querying #13830

Merged

Conversation

ZiheLiu
Copy link
Contributor

@ZiheLiu ZiheLiu commented Nov 22, 2022

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Which issues of this PR fixes :

This issue #3689 occurs again.
And maybe cause BE to be crashed.

Problem Summary(Required) :

#5056 changes the behaviour of stale rowsets.

  • Before it, the stale rowset will be deleted, only when shared_ptr<RowSet>.use_count() is one.
  • However, after it, the stale rowset will be closed early, even if shared_ptr.use_count() is greater than one but _refs_by_reader is zero.

Therefore, we need acquire rowsets after capturing them at querying.

Because we need call release_rowsets after OlapScanPrepareOperator and OlapScanOperator are both destructed, we move the logic of capture_tablet_rowsets from OlapScanPrepareOperator to OlapScanContext.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr will affect users' behaviors
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • I have checked the version labels which the pr will be auto backported to target branch

@ZiheLiu
Copy link
Contributor Author

ZiheLiu commented Nov 22, 2022

@mergify backport branch-2.3 branch-2.4 branch-2.5

@mergify
Copy link
Contributor

mergify bot commented Nov 22, 2022

backport branch-2.3 branch-2.4 branch-2.5

✅ Backports have been created

@StarRocks StarRocks deleted a comment from mergify bot Nov 22, 2022
@ZiheLiu
Copy link
Contributor Author

ZiheLiu commented Nov 22, 2022

backport branch-2.3 branch-2.4 branch-2.5

branch-2.2 need to be cherry-picked manually.

@ZiheLiu
Copy link
Contributor Author

ZiheLiu commented Nov 22, 2022

run starrocks_clang-tidy

@ZiheLiu
Copy link
Contributor Author

ZiheLiu commented Nov 22, 2022

run starrocks_clang-tidy

@github-actions
Copy link

clang-tidy review says "All clean, LGTM! 👍"

@mofeiatwork mofeiatwork enabled auto-merge (squash) November 22, 2022 12:11
@wanpengfei-git wanpengfei-git added the Approved Ready to merge label Nov 22, 2022
@wanpengfei-git
Copy link
Collaborator

run starrocks_admit_test

@ZiheLiu ZiheLiu merged commit c0a1b94 into StarRocks:main Nov 22, 2022
@ZiheLiu ZiheLiu deleted the fix/main/acquire_rowset_in_scan_node branch November 22, 2022 16:16
@wanpengfei-git
Copy link
Collaborator

@Mergifyio backport branch-2.5

@github-actions github-actions bot removed the 2.5 label Nov 22, 2022
@wanpengfei-git
Copy link
Collaborator

@Mergifyio backport branch-2.4

@github-actions github-actions bot removed the 2.4 label Nov 22, 2022
@wanpengfei-git
Copy link
Collaborator

@Mergifyio backport branch-2.3

@github-actions github-actions bot removed Approved Ready to merge be-build 2.3 labels Nov 22, 2022
@wanpengfei-git
Copy link
Collaborator

@Mergifyio backport branch-2.2

@github-actions github-actions bot removed the 2.2 label Nov 22, 2022
mergify bot pushed a commit that referenced this pull request Nov 22, 2022
(cherry picked from commit c0a1b94)

# Conflicts:
#	be/src/exec/pipeline/scan/olap_scan_context.cpp
mergify bot pushed a commit that referenced this pull request Nov 22, 2022
mergify bot pushed a commit that referenced this pull request Nov 22, 2022
@mergify
Copy link
Contributor

mergify bot commented Nov 22, 2022

backport branch-2.5

✅ Backports have been created

@mergify
Copy link
Contributor

mergify bot commented Nov 22, 2022

backport branch-2.4

✅ Backports have been created

@mergify
Copy link
Contributor

mergify bot commented Nov 22, 2022

backport branch-2.3

✅ Backports have been created

@mergify
Copy link
Contributor

mergify bot commented Nov 22, 2022

backport branch-2.2

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Nov 22, 2022
(cherry picked from commit c0a1b94)

# Conflicts:
#	be/src/exec/pipeline/scan/olap_scan_context.cpp
#	be/src/exec/pipeline/scan/olap_scan_context.h
#	be/src/exec/pipeline/scan/olap_scan_prepare_operator.cpp
#	be/src/exec/pipeline/scan/olap_scan_prepare_operator.h
@sonarcloud
Copy link

sonarcloud bot commented Nov 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
1.4% 1.4% Duplication

@github-actions
Copy link

clang-tidy review says "All clean, LGTM! 👍"

wanpengfei-git pushed a commit that referenced this pull request Nov 23, 2022
wanpengfei-git pushed a commit that referenced this pull request Nov 23, 2022
ZiheLiu added a commit to ZiheLiu/starrocks that referenced this pull request Nov 24, 2022
ZiheLiu added a commit to ZiheLiu/starrocks that referenced this pull request Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants