Skip to content

[Bugfix] (ROLLUP) fix the coredump when add rollup by link schema change#15654

Merged
yiguolei merged 4 commits intoapache:masterfrom
Lchangliang:schema_change_fix
Jan 14, 2023
Merged

[Bugfix] (ROLLUP) fix the coredump when add rollup by link schema change#15654
yiguolei merged 4 commits intoapache:masterfrom
Lchangliang:schema_change_fix

Conversation

@Lchangliang
Copy link
Copy Markdown
Contributor

@Lchangliang Lchangliang commented Jan 5, 2023

Proposed changes

Issue Number: close #15593

Problem summary

example:

CREATE TABLE IF NOT EXISTS sale (
                `user_id` LARGEINT NOT NULL COMMENT "用户id",
                `date` DATE NOT NULL COMMENT "数据灌入日期时间",
                `age` SMALLINT COMMENT "用户年龄",
                `sex` TINYINT COMMENT "用户性别",
                `city` VARCHAR(20) DEFAULT "beijing "COMMENT "用户所在城市",
                `last_visit_date` DATETIME DEFAULT "1970-01-01 00:00:00" COMMENT "用户最后一次访问时间",
                `last_update_date` DATETIME DEFAULT "1970-01-01 00:00:00" COMMENT "用户最后一次更新时间",
                `last_visit_date_not_null` DATETIME NOT NULL DEFAULT "1970-01-01 00:00:00" COMMENT "用户最后一次访问时间",
                `cost` BIGINT DEFAULT "0" COMMENT "用户总消费",
                `max_dwell_time` INT DEFAULT "0" COMMENT "用户最大停留时间",
                `min_dwell_time` INT DEFAULT "99999" COMMENT "用户最小停留时间")
            UNIQUE KEY(`user_id`, `date`, `age`, `sex`) DISTRIBUTED BY HASH(`user_id`)
            BUCKETS 1
            PROPERTIES ( "replication_num" = "1", "light_schema_change" = "true");

insert datas

ALTER TABLE sale ADD ROLLUP rollup(`user_id`,`date`,`age`, `sex`, `cost`);

select rollup
coredump

image

Because of the rollup has the same keys and the keys's order is same, BE will do linked schema change. The base tablet's segments will link to the new rollup tablet. But the unique id from the base tablet is starting from 0 and as the rollup tablet also. In this case, the unique id 4 in the base table is column 'city', but in the rollup tablet is 'cost'. It will decode the varcode page to bigint page so that be coredump. It needs to be rejected.

I think that if a rollup add by link schema change, it means this rollup is redundant. It brings no additional revenue and wastes storage space. So It needs to be rejected.

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • No

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 5, 2023

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

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 5, 2023

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

@hello-stephen
Copy link
Copy Markdown
Contributor

hello-stephen commented Jan 5, 2023

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 35.36 seconds
load time: 499 seconds
storage size: 17122758211 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230112103853_clickbench_pr_78520.html

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 6, 2023

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 8, 2023

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

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 8, 2023

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 9, 2023

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

3 similar comments
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 9, 2023

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 9, 2023

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

@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

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

@dataroaring dataroaring self-assigned this Jan 13, 2023
Copy link
Copy Markdown
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jan 13, 2023
@github-actions
Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

Copy link
Copy Markdown
Contributor

@yiguolei yiguolei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yiguolei yiguolei merged commit 313e14d into apache:master Jan 14, 2023
morningman pushed a commit that referenced this pull request Jan 16, 2023
…nge (#15654)

Because of the rollup has the same keys and the keys's order is same, BE will do linked schema change. The base tablet's segments will link to the new rollup tablet. But the unique id from the base tablet is starting from 0 and as the rollup tablet also. In this case, the unique id 4 in the base table is column 'city', but in the rollup tablet is 'cost'. It will decode the varcode page to bigint page so that be coredump. It needs to be rejected.

I think that if a rollup add by link schema change, it means this rollup is redundant. It brings no additional revenue and wastes storage space. So It needs to be rejected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/1.2.2-merged kind/test reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [daily] be core when query with rollup

5 participants