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] Be maybe crash when we do schema change for primary key table with separate primary key and sort key #20035

Merged
merged 6 commits into from
Mar 28, 2023

Conversation

sevev
Copy link
Contributor

@sevev sevev commented Mar 22, 2023

What type of PR is this:

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

Which issues of this PR fixes :

Fixes #

Problem Summary(Required) :

If we create a primary key table with a separate primary key and sort key, BE maybe crash if we do a schema change in the table(add column/drop column)
The reason is that we use column id as the sort key index in the tablet, and we only support reordering the sort key in this pr(#13642) which assumes the number of columns in the origin tablet and new tablet during schema change is the same. However, if we add/drop a column, the sort key index in the new tablet will be different from the origin tablet and we will execute reorder which may cause BE crash.

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

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto backported to target branch
    • 3.0
    • 2.5
    • 2.4
    • 2.3

be/src/storage/tablet_manager.cpp Outdated Show resolved Hide resolved
be/src/storage/schema_change.cpp Show resolved Hide resolved
@decster
Copy link
Contributor

decster commented Mar 23, 2023

Error:  COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
Error:  /home/runner/work/starrocks/starrocks/fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeJobV2.java:[331,25] cannot find symbol

Signed-off-by: zhangqiang <[email protected]>
Signed-off-by: zhangqiang <[email protected]>
@sevev
Copy link
Contributor Author

sevev commented Mar 23, 2023

Error:  COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
Error:  /home/runner/work/starrocks/starrocks/fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeJobV2.java:[331,25] cannot find symbol

done, fixed.

decster
decster previously approved these changes Mar 23, 2023
Signed-off-by: zhangqiang <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Mar 24, 2023

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@sevev
Copy link
Contributor Author

sevev commented Mar 27, 2023

@chaoyli @decster PTAL~

@decster decster merged commit 82f9d31 into StarRocks:main Mar 28, 2023
@wanpengfei-git
Copy link
Collaborator

@Mergifyio backport branch-3.0

@wanpengfei-git
Copy link
Collaborator

@Mergifyio backport branch-2.5

@mergify
Copy link
Contributor

mergify bot commented Mar 28, 2023

backport branch-3.0

✅ Backports have been created

@github-actions github-actions bot removed the 2.5 label Mar 28, 2023
@mergify
Copy link
Contributor

mergify bot commented Mar 28, 2023

backport branch-2.5

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Mar 28, 2023
…e with separate primary key and sort key (#20035)

If we create a primary key table with a separate primary key and sort key, BE maybe crash if we do a schema change in the table(add column/drop column)
The reason is that we use column id as the sort key index in the tablet, and we only support reordering the sort key in this pr(#13642) which assumes the number of columns in the origin tablet and new tablet during schema change is the same. However, if we add/drop a column, the sort key index in the new tablet will be different from the origin tablet and we will execute reorder which may cause BE crash.

Signed-off-by: zhangqiang <[email protected]>
(cherry picked from commit 82f9d31)
mergify bot pushed a commit that referenced this pull request Mar 28, 2023
…e with separate primary key and sort key (#20035)

If we create a primary key table with a separate primary key and sort key, BE maybe crash if we do a schema change in the table(add column/drop column)
The reason is that we use column id as the sort key index in the tablet, and we only support reordering the sort key in this pr(#13642) which assumes the number of columns in the origin tablet and new tablet during schema change is the same. However, if we add/drop a column, the sort key index in the new tablet will be different from the origin tablet and we will execute reorder which may cause BE crash.

Signed-off-by: zhangqiang <[email protected]>
(cherry picked from commit 82f9d31)
wanpengfei-git pushed a commit that referenced this pull request Mar 29, 2023
…e with separate primary key and sort key (#20035)

If we create a primary key table with a separate primary key and sort key, BE maybe crash if we do a schema change in the table(add column/drop column)
The reason is that we use column id as the sort key index in the tablet, and we only support reordering the sort key in this pr(#13642) which assumes the number of columns in the origin tablet and new tablet during schema change is the same. However, if we add/drop a column, the sort key index in the new tablet will be different from the origin tablet and we will execute reorder which may cause BE crash.

Signed-off-by: zhangqiang <[email protected]>
(cherry picked from commit 82f9d31)
numbernumberone pushed a commit to numbernumberone/starrocks that referenced this pull request May 31, 2023
…e with separate primary key and sort key (StarRocks#20035)

If we create a primary key table with a separate primary key and sort key, BE maybe crash if we do a schema change in the table(add column/drop column)
The reason is that we use column id as the sort key index in the tablet, and we only support reordering the sort key in this pr(StarRocks#13642) which assumes the number of columns in the origin tablet and new tablet during schema change is the same. However, if we add/drop a column, the sort key index in the new tablet will be different from the origin tablet and we will execute reorder which may cause BE crash.

Signed-off-by: zhangqiang <[email protected]>
abc982627271 pushed a commit to abc982627271/starrocks that referenced this pull request Jun 5, 2023
…e with separate primary key and sort key (StarRocks#20035)

If we create a primary key table with a separate primary key and sort key, BE maybe crash if we do a schema change in the table(add column/drop column)
The reason is that we use column id as the sort key index in the tablet, and we only support reordering the sort key in this pr(StarRocks#13642) which assumes the number of columns in the origin tablet and new tablet during schema change is the same. However, if we add/drop a column, the sort key index in the new tablet will be different from the origin tablet and we will execute reorder which may cause BE crash.

Signed-off-by: zhangqiang <[email protected]>
@sevev sevev deleted the lost_order_info_after_alter branch August 7, 2023 01:51
sevev added a commit to sevev/starrocks that referenced this pull request Dec 7, 2023
…e with separate primary key and sort key (StarRocks#20035)

If we create a primary key table with a separate primary key and sort key, BE maybe crash if we do a schema change in the table(add column/drop column)
The reason is that we use column id as the sort key index in the tablet, and we only support reordering the sort key in this pr(StarRocks#13642) which assumes the number of columns in the origin tablet and new tablet during schema change is the same. However, if we add/drop a column, the sort key index in the new tablet will be different from the origin tablet and we will execute reorder which may cause BE crash.

Signed-off-by: zhangqiang <[email protected]>
wanpengfei-git pushed a commit that referenced this pull request Dec 8, 2023
…ange for primary key table with separate primary key and sort key (#20035) (#36636)

Signed-off-by: zhangqiang <[email protected]>
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.

4 participants