fix(compactor): Compactor potential oom risk of builder #16802

Li0k · 2024-05-17T05:50:25Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

The following PR addresses the issue of out-of-memory (OOM) errors caused by inaccurate memory estimation in the compactor component. With the implementation of StreamingUploader and SstableBuilder, we will now pass the buffer to StreamingUploader and trigger an asynchronous upload when the buffer size is met. We will then harvest these join handles when compaction is completed. However, if the object store service capability is poor, a large amount of memory may be consumed by in-flight requests, leading to OOM errors in the compactor component. Asynchronous operations may bring better pipeline effects and higher throughput, but it may worsen things in the above situation. Therefore, some changes were made in this PR

change

Change the memory estimation of the compact task (reserved whole sst capacity)
Wait for uploader finish synchronously when switching builder.

refactor

use batch cancel for heartbeat cancel_task
introduce the new compact_task HeartbeatProgressCacnel to distinguish lack of visible progress and no heartbeat

related to #15946

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added test labels as necessary. See details.
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

…nto li0k/test_builder_oom

Li0k · 2024-05-17T06:22:30Z

rerun oom test with minio nvmessd https://buildkite.com/risingwave-test/nexmark-benchmark/builds/3695#018f80e7-bd5e-41a4-b7ba-dab0bf80c697
nexmark

…nto li0k/test_builder_oom

hzxa21

Can you share the result of bench_multi_builder prior and after this PR.

src/meta/src/hummock/compactor_manager.rs

hzxa21 · 2024-05-20T08:51:48Z

src/storage/src/hummock/compactor/mod.rs

        metrics: Arc<CompactorMetrics>,
        task_progress: Option<Arc<TaskProgress>>,
-        split_table_outputs: Vec<SplitTableOutput>,
+        ssts: &Vec<LocalSstableInfo>,


What benefit can we get if we pass &Vec<LocalSstableInfo> instead of Vec<SplitTableOutput> here? After this PR, we need to create a new vec via collect every time before calling report_progress, which seems like burden and overhead that can be avoided.

Previously, when report_progress was called, the vec was built from scratch and the LocalSstableInfo was taken from the SplitTableOutput, which is similar.

…nto li0k/test_builder_oom

Li0k · 2024-05-21T13:41:47Z

Can you share the result of bench_multi_builder prior and after this PR.

Test with local minio-ssd

main

bench_multi_builder/bench_batch_upload_4mb                                                                          
                        time:   [2.6181 s 2.6618 s 2.7245 s]
                        change: [-14.652% -10.122% -5.3304%] (p = 0.00 < 0.05)
                        Performance has improved.

bench_multi_builder/bench_streaming_upload_4mb                                                                           
                        time:   [787.01 ms 872.17 ms 975.67 ms]

bench_multi_builder/bench_batch_upload_32mb                                                                          
                        time:   [2.5894 s 2.6745 s 2.7805 s]

bench_multi_builder/bench_streaming_upload_32mb                                                                           
                        time:   [964.44 ms 1.0390 s 1.1069 s]

bench_multi_builder/bench_batch_upload_64mb                                                                          
                        time:   [2.6182 s 2.6526 s 2.6930 s]

bench_multi_builder/bench_streaming_upload_64mb                                                                           
                        time:   [1.6306 s 1.7731 s 1.8514 s]

bench_multi_builder/bench_batch_upload_128mb                                                                          
                        time:   [2.7079 s 2.7566 s 2.8151 s]

bench_multi_builder/bench_streaming_upload_128mb                                                                           
                        time:   [767.54 ms 823.25 ms 971.25 ms]

bench_multi_builder/bench_batch_upload_256mb                                                                          
                        time:   [2.9362 s 2.9446 s 2.9561 s]

bench_multi_builder/bench_streaming_upload_256mb                                                                           
                        time:   [787.30 ms 802.22 ms 827.87 ms]

pr

bench_multi_builder/bench_batch_upload_4mb                                                                          
                        time:   [4.1697 s 4.1790 s 4.1889 s]
                        change: [+53.336% +57.003% +59.659%] (p = 0.00 < 0.05)
                        Performance has regressed.

Benchmarking bench_multi_builder/bench_streaming_upload_4mb: Warming up for 3.0000 s
Warning: Unable to complete 10 samples in 60.0s. You may wish to increase target time to 107.7s or enable flat sampling.
bench_multi_builder/bench_streaming_upload_4mb                                                                          
                        time:   [1.9721 s 2.0226 s 2.0809 s]
                        change: [+117.88% +134.37% +151.32%] (p = 0.00 < 0.05)
                        Performance has regressed.

bench_multi_builder/bench_batch_upload_32mb                                                                          
                        time:   [3.9310 s 3.9432 s 3.9578 s]
                        change: [+41.753% +47.433% +52.305%] (p = 0.00 < 0.05)
                        Performance has regressed.

Benchmarking bench_multi_builder/bench_streaming_upload_32mb: Warming up for 3.0000 s
Warning: Unable to complete 10 samples in 60.0s. You may wish to increase target time to 69.6s or enable flat sampling.
bench_multi_builder/bench_streaming_upload_32mb                                                                          
                        time:   [1.3364 s 1.3574 s 1.3694 s]
                        change: [+22.254% +29.773% +38.817%] (p = 0.00 < 0.05)
                        Performance has regressed.

bench_multi_builder/bench_batch_upload_64mb                                                                          
                        time:   [4.1482 s 4.1621 s 4.1787 s]
                        change: [+54.469% +56.908% +59.020%] (p = 0.00 < 0.05)
                        Performance has regressed.

bench_multi_builder/bench_streaming_upload_64mb                                                                           
                        time:   [1.0002 s 1.0386 s 1.1139 s]
                        change: [-39.964% -27.946% -8.3196%] (p = 0.02 < 0.05)
                        Performance has improved.

bench_multi_builder/bench_batch_upload_128mb                                                                          
                        time:   [3.9774 s 3.9868 s 3.9959 s]
                        change: [+41.640% +44.627% +47.235%] (p = 0.00 < 0.05)
                        Performance has regressed.

bench_multi_builder/bench_streaming_upload_128mb                                                                           
                        time:   [892.23 ms 910.19 ms 925.58 ms]
                        change: [-22.538% -5.1263% +13.777%] (p = 0.69 > 0.05)
                        No change in performance detected.

bench_multi_builder/bench_batch_upload_256mb                                                                          
                        time:   [3.7851 s 3.7950 s 3.8038 s]
                        change: [+28.301% +28.879% +29.389%] (p = 0.00 < 0.05)
                        Performance has regressed.

bench_multi_builder/bench_streaming_upload_256mb                                                                           
                        time:   [810.20 ms 861.72 ms 924.04 ms]
                        change: [-1.9790% +5.2835% +12.629%] (p = 0.19 > 0.05)
                        No change in performance detected.

…nto li0k/test_builder_oom

zwang28 · 2024-05-23T11:38:32Z

Wait for uploader finish synchronously when switching builder.

Intuitively this change will significantly slow down SST building.

Li0k · 2024-05-27T07:01:35Z

Wait for uploader finish synchronously when switching builder.

Intuitively this change will significantly slow down SST building.

Yes, this pr prevents sst from being generated asynchronously, which will slow down the execution pipeline, and mirco bench has demonstrated significant performance degradation (only mircobench but nexmark). But I haven't found a better way yet, what do you think? @zwang28

Little-Wallace · 2024-05-31T09:31:59Z

proto/hummock.proto

@@ -328,6 +328,7 @@ message CompactTask {
    JOIN_HANDLE_FAILED = 11;
    TRACK_SST_OBJECT_ID_FAILED = 12;
    NO_AVAIL_CPU_RESOURCE_CANCELED = 13;
+    HEARTBEAT_PROGRESS_CANCELED = 14;


Is it unused ?

Li0k added 2 commits May 16, 2024 14:38

test(compactor): test compactor oom with minio

7063dee

fix(compactor): fix

b90250c

github-actions bot added type/fix Bug fix ci/run-e2e-single-node-tests labels May 17, 2024

Li0k added 2 commits May 17, 2024 14:07

refactor(compactor): refactor

65e48ab

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

7d52211

…nto li0k/test_builder_oom

Li0k marked this pull request as ready for review May 17, 2024 06:21

Li0k requested review from Little-Wallace, hzxa21, zwang28 and MrCroxx and removed request for Little-Wallace May 17, 2024 06:21

Li0k added 5 commits May 17, 2024 14:45

fix(compactor): fix compile

80d2322

fix(compactor): fix compile

7218e99

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

0eefad9

…nto li0k/test_builder_oom

chore(doc): upgrade doc of retry

a8000d9

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

6808758

…nto li0k/test_builder_oom

hzxa21 reviewed May 20, 2024

View reviewed changes

fix(compactor): address comments

4bc3042

Li0k enabled auto-merge May 20, 2024 09:33

Li0k added 2 commits May 20, 2024 17:37

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

37f13c3

…nto li0k/test_builder_oom

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

07ec55b

…nto li0k/test_builder_oom

Li0k added 4 commits May 22, 2024 14:27

feat(compactor): support local test for bench_multi_builder.rs

85f2645

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

ec20727

…nto li0k/test_builder_oom

fix(compactor): fix clippy

de70c05

Merge branch 'main' of https://github.com/risingwavelabs/risingwave i…

b17b180

…nto li0k/test_builder_oom

Little-Wallace reviewed May 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(compactor): Compactor potential oom risk of builder #16802

fix(compactor): Compactor potential oom risk of builder #16802

Li0k commented May 17, 2024 •

edited

Li0k commented May 17, 2024

hzxa21 left a comment

hzxa21 May 20, 2024

Li0k May 20, 2024

Li0k commented May 21, 2024 •

edited

zwang28 commented May 23, 2024

Li0k commented May 27, 2024

Little-Wallace May 31, 2024

fix(compactor): Compactor potential oom risk of builder #16802

Are you sure you want to change the base?

fix(compactor): Compactor potential oom risk of builder #16802

Conversation

Li0k commented May 17, 2024 • edited

What's changed and what's your intention?

change

refactor

Checklist

Documentation

Release note

Li0k commented May 17, 2024

hzxa21 left a comment

Choose a reason for hiding this comment

hzxa21 May 20, 2024

Choose a reason for hiding this comment

Li0k May 20, 2024

Choose a reason for hiding this comment

Li0k commented May 21, 2024 • edited

zwang28 commented May 23, 2024

Li0k commented May 27, 2024

Little-Wallace May 31, 2024

Choose a reason for hiding this comment

Li0k commented May 17, 2024 •

edited

Li0k commented May 21, 2024 •

edited