-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[refactor](partial-update) Split partial update infos from tablet schema #25147
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
[refactor](partial-update) Split partial update infos from tablet schema #25147
Conversation
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.
clang-tidy made some suggestions
| // For UT | ||
| DeleteBitmapPtr get_delete_bitmap() { return _delete_bitmap; } | ||
|
|
||
| std::shared_ptr<PartialUpdateInfo> get_partial_update_info() const { |
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.
warning: function 'get_partial_update_info' should be marked [[nodiscard]] [modernize-use-nodiscard]
| std::shared_ptr<PartialUpdateInfo> get_partial_update_info() const { | |
| [[nodiscard]] std::shared_ptr<PartialUpdateInfo> get_partial_update_info() const { |
6a2aad7 to
dac6ff6
Compare
|
run buildall |
1cc37d2 to
ef6af23
Compare
ef6af23 to
a2298a6
Compare
|
run buildall |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
60503b6 to
dd5a2b8
Compare
|
run buildall |
|
TeamCity be ut coverage result: |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
1 similar comment
|
run buildall |
|
TeamCity be ut coverage result: |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
302be55 to
b6a40f2
Compare
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
liaoxin01
left a comment
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.
LGTM
1. remove deprecated comment on fields that is wrongly added in #25147. These fields will be used when coordinator BE send infos to executor BEs. They will only be used during RPC and will not be persisted. 2. eliminate some unnecessary copys.
1. remove deprecated comment on fields that is wrongly added in #25147. These fields will be used when coordinator BE send infos to executor BEs. They will only be used during RPC and will not be persisted. 2. eliminate some unnecessary copys.
1. remove deprecated comment on fields that is wrongly added in apache#25147. These fields will be used when coordinator BE send infos to executor BEs. They will only be used during RPC and will not be persisted. 2. eliminate some unnecessary copys.
…tmaps of the committed transactions are calculated by the compaction (apache#26556) a fix for apache#25147
…tmaps of the committed transactions are calculated by the compaction (apache#26556) a fix for apache#25147
1. remove deprecated comment on fields that is wrongly added in apache#25147. These fields will be used when coordinator BE send infos to executor BEs. They will only be used during RPC and will not be persisted. 2. eliminate some unnecessary copys.
…tmaps of the committed transactions are calculated by the compaction (apache#26556) a fix for apache#25147
…of BE restart after a partial update has commited (#38331) ## Proposed changes If a partial update has conflict with another load during publish phase, it should combine the two load's data into one to get the corrrect result. This procedure needs partial update info. But If BE crashed after the partial update load has committed, the partial update info will be missing becasuse it's not persisted and will not be restored in `DataDir::load()`. This PR persists partial update info in RocksDB before the txn is commited and remove it after the publish phase. Before #25147, partial update info is persisted with tablet_schema in RocksDB. #25147 split partial update info from tablet schema but forget to handle the persistence logic.
…of BE restart after a partial update has commited (#38331) ## Proposed changes If a partial update has conflict with another load during publish phase, it should combine the two load's data into one to get the corrrect result. This procedure needs partial update info. But If BE crashed after the partial update load has committed, the partial update info will be missing becasuse it's not persisted and will not be restored in `DataDir::load()`. This PR persists partial update info in RocksDB before the txn is commited and remove it after the publish phase. Before #25147, partial update info is persisted with tablet_schema in RocksDB. #25147 split partial update info from tablet schema but forget to handle the persistence logic.
…of BE restart after a partial update has commited (apache#38331) ## Proposed changes If a partial update has conflict with another load during publish phase, it should combine the two load's data into one to get the corrrect result. This procedure needs partial update info. But If BE crashed after the partial update load has committed, the partial update info will be missing becasuse it's not persisted and will not be restored in `DataDir::load()`. This PR persists partial update info in RocksDB before the txn is commited and remove it after the publish phase. Before apache#25147, partial update info is persisted with tablet_schema in RocksDB. apache#25147 split partial update info from tablet schema but forget to handle the persistence logic.
…of BE restart after a partial update has commited (#38331) ## Proposed changes If a partial update has conflict with another load during publish phase, it should combine the two load's data into one to get the corrrect result. This procedure needs partial update info. But If BE crashed after the partial update load has committed, the partial update info will be missing becasuse it's not persisted and will not be restored in `DataDir::load()`. This PR persists partial update info in RocksDB before the txn is commited and remove it after the publish phase. Before #25147, partial update info is persisted with tablet_schema in RocksDB. #25147 split partial update info from tablet schema but forget to handle the persistence logic.
Proposed changes
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...