Skip to content

Conversation

@bobhan1
Copy link
Contributor

@bobhan1 bobhan1 commented Nov 9, 2023

Proposed changes

This PR reverts the optimization for delete sign in #24011

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

@bobhan1
Copy link
Contributor Author

bobhan1 commented Nov 9, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2023

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

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.74% (8394/22847)
Line Coverage: 29.25% (68086/232744)
Region Coverage: 27.90% (35216/126242)
Branch Coverage: 24.70% (18005/72892)
Coverage Report: http://coverage.selectdb-in.cc/coverage/21f6a429d179889db6b168b360e2e9bf3321bb0a_21f6a429d179889db6b168b360e2e9bf3321bb0a/report/index.html

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.74 seconds
stream load tsv: 560 seconds loaded 74807831229 Bytes, about 127 MB/s
stream load json: 21 seconds loaded 2358488459 Bytes, about 107 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 29.1 seconds inserted 10000000 Rows, about 343K ops/s
storage size: 17162702103 Bytes

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
Tpch sf100 test result on commit 21f6a429d179889db6b168b360e2e9bf3321bb0a, data reload: false

run tpch-sf100 query with default conf and session variables
q1	5185	5109	5039	5039
q2	368	184	203	184
q3	2075	2090	2080	2080
q4	1487	1426	1420	1420
q5	4156	4188	4132	4132
q6	261	138	134	134
q7	2105	1589	1628	1589
q8	2762	2738	2760	2738
q9	10425	10394	10215	10215
q10	3498	3533	3561	3533
q11	381	256	250	250
q12	471	304	305	304
q13	4552	4060	4087	4060
q14	334	292	294	292
q15	631	567	570	567
q16	697	618	595	595
q17	1155	1084	1091	1084
q18	7718	7330	7402	7330
q19	1703	1791	1699	1699
q20	597	377	345	345
q21	4987	4607	4573	4573
q22	531	437	429	429
Total cold run time: 56079 ms
Total hot run time: 52592 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4964	4992	4985	4985
q2	367	287	245	245
q3	4100	4000	3972	3972
q4	2778	2802	2786	2786
q5	6548	6486	6517	6486
q6	246	128	127	127
q7	3158	2708	2740	2708
q8	4906	4775	4767	4767
q9	17758	17748	17809	17748
q10	4078	4165	4156	4156
q11	829	626	655	626
q12	1015	877	802	802
q13	4313	3951	3891	3891
q14	406	368	367	367
q15	648	588	593	588
q16	754	690	699	690
q17	3969	3960	3902	3902
q18	9480	9356	9306	9306
q19	1855	1774	1773	1773
q20	2407	2053	2062	2053
q21	8843	8832	8651	8651
q22	953	879	886	879
Total cold run time: 84375 ms
Total hot run time: 81508 ms

dataroaring
dataroaring previously approved these changes Nov 10, 2023
Copy link
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 Nov 10, 2023
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

Copy link
Contributor

@liaoxin01 liaoxin01 left a comment

Choose a reason for hiding this comment

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

LGTM

@bobhan1 bobhan1 force-pushed the remove-delete-sign-delete-bitmap-mark branch from 21f6a42 to 04c7244 Compare November 10, 2023 02:53
@bobhan1
Copy link
Contributor Author

bobhan1 commented Nov 10, 2023

run buildall

@bobhan1 bobhan1 requested a review from zhannngchen November 10, 2023 02:54
@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Nov 10, 2023
@github-actions
Copy link
Contributor

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

Copy link
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 Nov 10, 2023
@github-actions
Copy link
Contributor

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

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.77% (8404/22856)
Line Coverage: 29.28% (68137/232699)
Region Coverage: 27.92% (35237/126200)
Branch Coverage: 24.73% (18018/72858)
Coverage Report: http://coverage.selectdb-in.cc/coverage/04c7244dedf19aed9a4dd1ec67584c549405d1c8_04c7244dedf19aed9a4dd1ec67584c549405d1c8/report/index.html

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.61 seconds
stream load tsv: 555 seconds loaded 74807831229 Bytes, about 128 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.2 seconds inserted 10000000 Rows, about 342K ops/s
storage size: 17162389997 Bytes

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
Tpch sf100 test result on commit 04c7244dedf19aed9a4dd1ec67584c549405d1c8, data reload: false

run tpch-sf100 query with default conf and session variables
q1	5192	5078	5227	5078
q2	375	219	198	198
q3	2095	2120	2049	2049
q4	1471	1435	1426	1426
q5	4136	4189	4122	4122
q6	255	133	134	133
q7	2085	1629	1618	1618
q8	2782	2755	2773	2755
q9	10412	10286	10312	10286
q10	3471	3570	3567	3567
q11	375	258	247	247
q12	461	303	306	303
q13	4535	4107	4108	4107
q14	329	301	291	291
q15	647	574	574	574
q16	692	619	601	601
q17	1134	1104	1113	1104
q18	7832	7425	7466	7425
q19	1728	1698	1712	1698
q20	609	365	372	365
q21	4951	4596	4568	4568
q22	532	442	440	440
Total cold run time: 56099 ms
Total hot run time: 52955 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	5025	4960	5055	4960
q2	346	244	261	244
q3	4000	3938	3994	3938
q4	2792	2757	2783	2757
q5	6492	6460	6498	6460
q6	245	130	128	128
q7	3158	2760	2725	2725
q8	4811	4865	4782	4782
q9	17840	17605	17765	17605
q10	4101	4166	4175	4166
q11	775	655	654	654
q12	990	827	849	827
q13	4302	3897	3901	3897
q14	396	356	362	356
q15	654	575	567	567
q16	786	692	683	683
q17	4013	3947	3938	3938
q18	9503	9188	9258	9188
q19	1887	1780	1783	1780
q20	2383	2042	2012	2012
q21	8822	8816	8719	8719
q22	964	874	871	871
Total cold run time: 84285 ms
Total hot run time: 81257 ms

@bobhan1
Copy link
Contributor Author

bobhan1 commented Nov 10, 2023

run pipelinex_p0

XuJianxu pushed a commit to XuJianxu/doris that referenced this pull request Dec 14, 2023
…ows with delete sign when sequence column doesn't exist (apache#26721)
dataroaring pushed a commit that referenced this pull request Jun 14, 2024
…rows with delete sign (#36210)

Issue Number: close #34296

1. When partial update filling in the missing fields, if a load job
previously wrote data with a delete sign, it will also read out the data
in the column with the delete sign, so that the newly written data will
also become invisible
2. This problem was fixed in #24877, but was introduced again in #26721,
and was never found because the case was changed to the wrong output in
#26721.
3. The fix in #24877 didn't take into account the handling of concurrent
conflicts in the publish phase, the current PR adds this part of the
handling, and adds the corresponding case.
dataroaring pushed a commit that referenced this pull request Jun 17, 2024
…rows with delete sign (#36210)

Issue Number: close #34296

1. When partial update filling in the missing fields, if a load job
previously wrote data with a delete sign, it will also read out the data
in the column with the delete sign, so that the newly written data will
also become invisible
2. This problem was fixed in #24877, but was introduced again in #26721,
and was never found because the case was changed to the wrong output in
#26721.
3. The fix in #24877 didn't take into account the handling of concurrent
conflicts in the publish phase, the current PR adds this part of the
handling, and adds the corresponding case.
zhannngchen added a commit to zhannngchen/incubator-doris that referenced this pull request Jun 24, 2024
…rows with delete sign (apache#36210)

Issue Number: close apache#34296

1. When partial update filling in the missing fields, if a load job
previously wrote data with a delete sign, it will also read out the data
in the column with the delete sign, so that the newly written data will
also become invisible
2. This problem was fixed in apache#24877, but was introduced again in apache#26721,
and was never found because the case was changed to the wrong output in
3. The fix in apache#24877 didn't take into account the handling of concurrent
conflicts in the publish phase, the current PR adds this part of the
handling, and adds the corresponding case.
zhannngchen added a commit to zhannngchen/incubator-doris that referenced this pull request Jun 24, 2024
…rows with delete sign (apache#36210)

Issue Number: close apache#34296

1. When partial update filling in the missing fields, if a load job
previously wrote data with a delete sign, it will also read out the data
in the column with the delete sign, so that the newly written data will
also become invisible
2. This problem was fixed in apache#24877, but was introduced again in apache#26721,
and was never found because the case was changed to the wrong output in
3. The fix in apache#24877 didn't take into account the handling of concurrent
conflicts in the publish phase, the current PR adds this part of the
handling, and adds the corresponding case.
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/2.0.4-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants