[refactor](partial-update) Split partial update infos from tablet schema by bobhan1 · Pull Request #25147 · apache/doris

bobhan1 · 2023-10-09T02:53:12Z

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...

github-actions

clang-tidy made some suggestions

github-actions · 2023-10-09T03:02:00Z

 // For UT
 DeleteBitmapPtr get_delete_bitmap() { return _delete_bitmap; }

+ std::shared_ptr<PartialUpdateInfo> get_partial_update_info() const {


warning: function 'get_partial_update_info' should be marked [[nodiscard]] [modernize-use-nodiscard]
Suggested change
std::shared_ptr<PartialUpdateInfo>get_partial_update_info() const {
[[nodiscard]] std::shared_ptr<PartialUpdateInfo>get_partial_update_info() const {

bobhan1 · 2023-10-09T04:20:10Z

run buildall

bobhan1 · 2023-10-09T10:11:25Z

run buildall

doris-robot · 2023-10-09T11:28:29Z

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.81 seconds
stream load tsv: 559 seconds loaded 74807831229 Bytes, about 127 MB/s
stream load json: 21 seconds loaded 2358488459 Bytes, about 107 MB/s
stream load orc: 64 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 31 seconds loaded 861443392 Bytes, about 26 MB/s
insert into select: 29.0 seconds inserted 10000000 Rows, about 344K ops/s
storage size: 17162259983 Bytes

bobhan1 · 2023-10-10T03:20:37Z

run buildall

doris-robot · 2023-10-10T04:31:18Z

TeamCity be ut coverage result:
Function Coverage: 36.30% (8148/22448)
Line Coverage: 28.44% (65181/229166)
Region Coverage: 27.37% (33770/123390)
Branch Coverage: 24.04% (17205/71560)
Coverage Report: http://coverage.selectdb-in.cc/coverage/dd5a2b817f88873434b1efb9b6520f6b3d581192_dd5a2b817f88873434b1efb9b6520f6b3d581192/report/index.html

doris-robot · 2023-10-10T04:41:17Z

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.5 seconds
stream load tsv: 574 seconds loaded 74807831229 Bytes, about 124 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 29.0 seconds inserted 10000000 Rows, about 344K ops/s
storage size: 17162466608 Bytes

bobhan1 · 2023-10-10T06:12:04Z

run buildall

bobhan1 · 2023-10-10T06:12:05Z

run buildall

doris-robot · 2023-10-10T06:55:52Z

TeamCity be ut coverage result:
Function Coverage: 36.28% (8148/22456)
Line Coverage: 28.43% (65179/229257)
Region Coverage: 27.36% (33776/123459)
Branch Coverage: 24.03% (17205/71612)
Coverage Report: http://coverage.selectdb-in.cc/coverage/302be55f489671f0e5e60184d5ab0f480ea6af6a_302be55f489671f0e5e60184d5ab0f480ea6af6a/report/index.html

doris-robot · 2023-10-10T07:18:16Z

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.57 seconds
stream load tsv: 554 seconds loaded 74807831229 Bytes, about 128 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 64 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 29.0 seconds inserted 10000000 Rows, about 344K ops/s
storage size: 17162128127 Bytes

github-actions · 2023-10-17T04:09:20Z

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

github-actions · 2023-10-17T04:09:23Z

PR approved by anyone and no changes requested.

liaoxin01

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.

…fos from tablet schema #25147" (#25754)

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.

…ema (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 (#26556) a fix for #25147

…tmaps of the committed transactions are calculated by the compaction (apache#26556) a fix for apache#25147

…ema (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 (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.

…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.

github-actionsBot reviewed Oct 9, 2023
View reviewed changes

bobhan1 force-pushed the split-out-partial-update-infos-from-tablet-schema branch 5 times, most recently from 6a2aad7 to dac6ff6Compare October 9, 2023 04:19

bobhan1 force-pushed the split-out-partial-update-infos-from-tablet-schema branch 7 times, most recently from 1cc37d2 to ef6af23Compare October 9, 2023 08:57

bobhan1 marked this pull request as ready for review October 9, 2023 08:59

bobhan1 force-pushed the split-out-partial-update-infos-from-tablet-schema branch from ef6af23 to a2298a6Compare October 9, 2023 09:05

bobhan1 changed the title ~~[enhancement] split partial update infos from tablet schema~~[enhancement](merge-on-write) Split partial update infos from tablet schemaOct 9, 2023

bobhan1 force-pushed the split-out-partial-update-infos-from-tablet-schema branch 3 times, most recently from 60503b6 to dd5a2b8Compare October 10, 2023 03:20

bobhan1 force-pushed the split-out-partial-update-infos-from-tablet-schema branch from 302be55 to b6a40f2Compare October 10, 2023 08:24

github-actionsBot added the reviewed label Oct 17, 2023

liaoxin01 approved these changes Oct 17, 2023
View reviewed changes

zhannngchen merged commit 1514f78 into apache:masterOct 17, 2023

xiaokang added the merge_conflict label Oct 22, 2023

bobhan1 mentioned this pull request Oct 23, 2023
[Fix](partial update) Fix wrong comment in .proto file #25742
Merged

bobhan1 added a commit to bobhan1/doris that referenced this pull request Oct 23, 2023
pickapache#25147
afd912c

bobhan1 mentioned this pull request Oct 23, 2023
[branch-2.0-pick] Picks "[refactor](partial-update) Split partial update infos from tablet schema #25147" #25754
Merged

bobhan1 added a commit to bobhan1/doris that referenced this pull request Oct 23, 2023
pickapache#25147
1e7d702

xiaokang pushed a commit that referenced this pull request Oct 23, 2023
[branch-2.0-pick] "[refactor](partial-update) Split partial update in…
331d2d3
…fos from tablet schema #25147" (#25754)

xiaokang added dev/2.0.3-merged and removed dev/2.0.3 labels Oct 23, 2023

dutyu pushed a commit to dutyu/doris that referenced this pull request Oct 28, 2023
[refactor](partial-update) Split partial update infos from tablet sch…
e8fd8d8
…ema (apache#25147)

bobhan1 mentioned this pull request Nov 8, 2023
[Fix](partial update) Fix partial update info loss when the delete bitmaps of the committed transactions are calculated by the compaction #26556
Merged

zhannngchen pushed a commit that referenced this pull request Nov 8, 2023
[Fix](partial update) Fix partial update info loss when the delete bi…
d0960ba
…tmaps of the committed transactions are calculated by the compaction (#26556) a fix for #25147

zhannngchen pushed a commit to zhannngchen/incubator-doris that referenced this pull request Nov 9, 2023
[Fix](partial update) Fix partial update info loss when the delete bi…
c68e431
…tmaps of the committed transactions are calculated by the compaction (apache#26556) a fix for apache#25147

seawinde pushed a commit to seawinde/doris that referenced this pull request Nov 13, 2023
[Fix](partial update) Fix partial update info loss when the delete bi…
78af9c5
…tmaps of the committed transactions are calculated by the compaction (apache#26556) a fix for apache#25147

XuJianxu pushed a commit to XuJianxu/doris that referenced this pull request Dec 14, 2023
[refactor](partial-update) Split partial update infos from tablet sch…
ac04713
…ema (apache#25147)

XuJianxu pushed a commit to XuJianxu/doris that referenced this pull request Dec 14, 2023
[Fix](partial update) Fix partial update info loss when the delete bi…
3b53988
…tmaps of the committed transactions are calculated by the compaction (apache#26556) a fix for apache#25147

bobhan1 mentioned this pull request Jul 24, 2024
[Fix](partial update) Persist partial_update_info in RocksDB in case of BE restart after a partial update has commited #38331
Merged

	std::shared_ptr<PartialUpdateInfo>get_partial_update_info() const {
	[[nodiscard]] std::shared_ptr<PartialUpdateInfo>get_partial_update_info() const {

Uh oh!

Conversation

bobhan1 commented Oct 9, 2023

Proposed changes

Further comments

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actionsBotOct 9, 2023

Choose a reason for hiding this comment

Uh oh!

bobhan1 commented Oct 9, 2023

Uh oh!

bobhan1 commented Oct 9, 2023

Uh oh!

doris-robot commented Oct 9, 2023

Uh oh!

bobhan1 commented Oct 10, 2023

Uh oh!

doris-robot commented Oct 10, 2023

Uh oh!

doris-robot commented Oct 10, 2023

Uh oh!

bobhan1 commented Oct 10, 2023

Uh oh!

bobhan1 commented Oct 10, 2023

Uh oh!

doris-robot commented Oct 10, 2023

Uh oh!

doris-robot commented Oct 10, 2023

Uh oh!

github-actionsBot commented Oct 17, 2023

Uh oh!

github-actionsBot commented Oct 17, 2023

Uh oh!

liaoxin01 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants