[Fix](memtable) fix `shrink_memtable_by_agg` should also update `_row…#28536

eldenmoon · 2023-12-17T17:15:44Z

…_in_blocks`

Otherwise using the stale _row_in_blocks will result in heap-buffer-overflow

==2695213==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62900122e210 at pc 0x56524744aecf bp 0x7f62c595ef7
0 sp 0x7f62c595ef68
READ of size 8 at 0x62900122e210 thread T1627 (MemTableFlushTh)
    #0 0x56524744aece in doris::vectorized::ColumnVector<long>::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_vector.cpp:378:33
    #1 0x5652472a7538 in doris::vectorized::ColumnNullable::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_nullable.cpp:310:25
    #2 0x56524782a62a in doris::vectorized::MutableBlock::add_rows(doris::vectorized::Block const*, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/core/block.cpp:961:14
    #3 0x565233f187ae in doris::MemTable::_put_into_output(doris::vectorized::Block&) /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:248:27
    #4 0x565233f1db66 in doris::MemTable::to_block() /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:496:13
    #5 0x565233efae60 in doris::FlushToken::_do_flush_memtable(doris::MemTable*, int, long*) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:121:62
    #6 0x565233efc8d6 in doris::FlushToken::_flush_memtable(doris::MemTable*, int, long) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:150:16
    #7 0x565233f0c5eb in doris::MemtableFlushTask::run() /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:58:23

Proposed changes

Issue Number: close #xxx

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

…_in_blocks` Otherwise using the stale `_row_in_blocks` will result in heap-use-after-free ``` ==2695213==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62900122e210 at pc 0x56524744aecf bp 0x7f62c595ef7 0 sp 0x7f62c595ef68 READ of size 8 at 0x62900122e210 thread T1627 (MemTableFlushTh) #0 0x56524744aece in doris::vectorized::ColumnVector<long>::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_vector.cpp:378:33 #1 0x5652472a7538 in doris::vectorized::ColumnNullable::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_nullable.cpp:310:25 apache#2 0x56524782a62a in doris::vectorized::MutableBlock::add_rows(doris::vectorized::Block const*, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/core/block.cpp:961:14 apache#3 0x565233f187ae in doris::MemTable::_put_into_output(doris::vectorized::Block&) /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:248:27 apache#4 0x565233f1db66 in doris::MemTable::to_block() /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:496:13 apache#5 0x565233efae60 in doris::FlushToken::_do_flush_memtable(doris::MemTable*, int, long*) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:121:62 apache#6 0x565233efc8d6 in doris::FlushToken::_flush_memtable(doris::MemTable*, int, long) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:150:16 apache#7 0x565233f0c5eb in doris::MemtableFlushTask::run() /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:58:23 ```

eldenmoon · 2023-12-17T17:17:38Z

run buildall

github-actions · 2023-12-17T17:23:23Z

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

doris-robot · 2023-12-17T17:51:33Z

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Tpch sf100 test result on commit 0a6e7b1442c68ddfdb10e60ff9d13d6391a5c378, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4682	4449	4519	4449
q2	357	165	162	162
q3	1462	1262	1238	1238
q4	1117	921	915	915
q5	3151	3153	3107	3107
q6	251	128	127	127
q7	1023	474	491	474
q8	2193	2191	2200	2191
q9	6744	6684	6697	6684
q10	3230	3259	3265	3259
q11	331	208	195	195
q12	351	208	209	208
q13	4570	3822	3806	3806
q14	245	211	212	211
q15	567	519	522	519
q16	443	382	386	382
q17	994	589	531	531
q18	7268	6976	6991	6976
q19	1525	1412	1444	1412
q20	541	305	301	301
q21	3044	2619	2660	2619
q22	347	275	282	275
Total cold run time: 44436 ms
Total hot run time: 40041 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4400	4412	4410	4410
q2	268	166	179	166
q3	3553	3535	3530	3530
q4	2373	2364	2357	2357
q5	5718	5744	5726	5726
q6	244	121	122	121
q7	2387	1849	1893	1849
q8	3527	3515	3523	3515
q9	9012	8989	8951	8951
q10	3905	4005	3989	3989
q11	506	397	371	371
q12	763	593	608	593
q13	4271	3594	3580	3580
q14	291	260	254	254
q15	574	526	518	518
q16	523	463	486	463
q17	1887	1859	1845	1845
q18	8696	8257	8273	8257
q19	1757	1745	1754	1745
q20	2250	1934	1940	1934
q21	6522	6140	6159	6140
q22	511	426	409	409
Total cold run time: 63938 ms
Total hot run time: 60723 ms

kaijchen

LGTM

github-actions · 2023-12-18T01:41:14Z

PR approved by anyone and no changes requested.

dataroaring

LGTM

github-actions · 2023-12-18T01:59:52Z

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

doris-robot · 2023-12-18T02:28:28Z

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Tpch sf100 test result on commit 0a6e7b1442c68ddfdb10e60ff9d13d6391a5c378, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4715	4497	4451	4451
q2	356	159	157	157
q3	1464	1259	1239	1239
q4	1115	909	897	897
q5	3132	3143	3161	3143
q6	243	126	128	126
q7	990	490	488	488
q8	2177	2266	2192	2192
q9	6672	6663	6646	6646
q10	3232	3282	3266	3266
q11	315	210	197	197
q12	351	214	201	201
q13	4552	3826	3815	3815
q14	240	212	209	209
q15	566	514	522	514
q16	442	392	393	392
q17	1009	675	591	591
q18	7148	7023	6923	6923
q19	1543	1427	1409	1409
q20	552	296	299	296
q21	3077	2647	2680	2647
q22	345	273	284	273
Total cold run time: 44236 ms
Total hot run time: 40072 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4361	4394	4391	4391
q2	273	164	171	164
q3	3548	3543	3499	3499
q4	2372	2369	2361	2361
q5	5720	5729	5727	5727
q6	246	120	123	120
q7	2384	1856	1885	1856
q8	3523	3513	3512	3512
q9	9022	8996	8993	8993
q10	3885	3976	3970	3970
q11	492	370	377	370
q12	767	595	624	595
q13	4313	3559	3555	3555
q14	282	258	260	258
q15	575	512	518	512
q16	509	448	457	448
q17	1869	1848	1858	1848
q18	8573	8283	8218	8218
q19	1719	1698	1723	1698
q20	2254	1942	1928	1928
q21	6507	6174	6170	6170
q22	508	431	425	425
Total cold run time: 63702 ms
Total hot run time: 60618 ms

…_in_blocks` (apache#28536) Otherwise using the stale `_row_in_blocks` will result in heap-buffer-overflow ``` ==2695213==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62900122e210 at pc 0x56524744aecf bp 0x7f62c595ef7 0 sp 0x7f62c595ef68 READ of size 8 at 0x62900122e210 thread T1627 (MemTableFlushTh) #0 0x56524744aece in doris::vectorized::ColumnVector<long>::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_vector.cpp:378:33 #1 0x5652472a7538 in doris::vectorized::ColumnNullable::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_nullable.cpp:310:25 apache#2 0x56524782a62a in doris::vectorized::MutableBlock::add_rows(doris::vectorized::Block const*, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/core/block.cpp:961:14 apache#3 0x565233f187ae in doris::MemTable::_put_into_output(doris::vectorized::Block&) /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:248:27 apache#4 0x565233f1db66 in doris::MemTable::to_block() /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:496:13 apache#5 0x565233efae60 in doris::FlushToken::_do_flush_memtable(doris::MemTable*, int, long*) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:121:62 apache#6 0x565233efc8d6 in doris::FlushToken::_flush_memtable(doris::MemTable*, int, long) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:150:16 apache#7 0x565233f0c5eb in doris::MemtableFlushTask::run() /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:58:23 ```

…28671) * [Fix](memtable) fix `shrink_memtable_by_agg` should also update `_row_in_blocks` (#28536) * [Fix](memtable) fix `shrink_memtable_by_agg` without duplicated keys (#28660)

…_in_blocks` (apache#28536) Otherwise using the stale `_row_in_blocks` will result in heap-buffer-overflow ``` ==2695213==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62900122e210 at pc 0x56524744aecf bp 0x7f62c595ef7 0 sp 0x7f62c595ef68 READ of size 8 at 0x62900122e210 thread T1627 (MemTableFlushTh) #0 0x56524744aece in doris::vectorized::ColumnVector<long>::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_vector.cpp:378:33 #1 0x5652472a7538 in doris::vectorized::ColumnNullable::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_nullable.cpp:310:25 #2 0x56524782a62a in doris::vectorized::MutableBlock::add_rows(doris::vectorized::Block const*, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/core/block.cpp:961:14 #3 0x565233f187ae in doris::MemTable::_put_into_output(doris::vectorized::Block&) /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:248:27 #4 0x565233f1db66 in doris::MemTable::to_block() /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:496:13 #5 0x565233efae60 in doris::FlushToken::_do_flush_memtable(doris::MemTable*, int, long*) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:121:62 #6 0x565233efc8d6 in doris::FlushToken::_flush_memtable(doris::MemTable*, int, long) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:150:16 #7 0x565233f0c5eb in doris::MemtableFlushTask::run() /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:58:23 ```

…_in_blocks` (apache#28536) Otherwise using the stale `_row_in_blocks` will result in heap-buffer-overflow ``` ==2695213==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62900122e210 at pc 0x56524744aecf bp 0x7f62c595ef7 0 sp 0x7f62c595ef68 READ of size 8 at 0x62900122e210 thread T1627 (MemTableFlushTh) #0 0x56524744aece in doris::vectorized::ColumnVector<long>::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_vector.cpp:378:33 #1 0x5652472a7538 in doris::vectorized::ColumnNullable::insert_indices_from(doris::vectorized::IColumn const&, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/columns/column_nullable.cpp:310:25 #2 0x56524782a62a in doris::vectorized::MutableBlock::add_rows(doris::vectorized::Block const*, unsigned int const*, unsigned int const*) /mnt/disk2/lihangyu/doris/be/src/vec/core/block.cpp:961:14 #3 0x565233f187ae in doris::MemTable::_put_into_output(doris::vectorized::Block&) /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:248:27 #4 0x565233f1db66 in doris::MemTable::to_block() /mnt/disk2/lihangyu/doris/be/src/olap/memtable.cpp:496:13 #5 0x565233efae60 in doris::FlushToken::_do_flush_memtable(doris::MemTable*, int, long*) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:121:62 #6 0x565233efc8d6 in doris::FlushToken::_flush_memtable(doris::MemTable*, int, long) /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:150:16 apache#7 0x565233f0c5eb in doris::MemtableFlushTask::run() /mnt/disk2/lihangyu/doris/be/src/olap/memtable_flush_executor.cpp:58:23 ```

kaijchen approved these changes Dec 18, 2023

View reviewed changes

github-actions bot added the reviewed label Dec 18, 2023

dataroaring approved these changes Dec 18, 2023

View reviewed changes

github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 18, 2023

eldenmoon merged commit d11365d into apache:master Dec 18, 2023

wm1581066 added the not-merge/2.0 do not merge into 2.0 branch label Dec 18, 2023

dataroaring added dev/2.0.4 and removed not-merge/2.0 do not merge into 2.0 branch labels Dec 18, 2023

wm1581066 added the p0_c label Dec 18, 2023

eldenmoon mentioned this pull request Dec 19, 2023

[Pick](branch-2.0) pick two PRs to fix memtable shrink #28671

Merged

xiaokang added dev/2.0.4-merged and removed dev/2.0.4 labels Dec 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix](memtable) fix `shrink_memtable_by_agg` should also update `_row…#28536

[Fix](memtable) fix `shrink_memtable_by_agg` should also update `_row…#28536
eldenmoon merged 1 commit intoapache:masterfrom
eldenmoon:fix-agg-shrink

eldenmoon commented Dec 17, 2023 •

edited

Loading

Uh oh!

eldenmoon commented Dec 17, 2023

Uh oh!

github-actions bot commented Dec 17, 2023

Uh oh!

doris-robot commented Dec 17, 2023

Uh oh!

kaijchen left a comment

Uh oh!

github-actions bot commented Dec 18, 2023

Uh oh!

dataroaring left a comment

Uh oh!

github-actions bot commented Dec 18, 2023

Uh oh!

doris-robot commented Dec 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments

Conversation

eldenmoon commented Dec 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Further comments

Uh oh!

eldenmoon commented Dec 17, 2023

Uh oh!

github-actions bot commented Dec 17, 2023

Uh oh!

doris-robot commented Dec 17, 2023

Uh oh!

kaijchen left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 18, 2023

Uh oh!

dataroaring left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 18, 2023

Uh oh!

doris-robot commented Dec 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments

eldenmoon commented Dec 17, 2023 •

edited

Loading