feat(compaction): limit memory usage for compaction read #4590

Little-Wallace · 2022-08-11T08:14:13Z

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

LRU-Cache can not limit memory strictly, although we can make insert LRU-Cache fail when memory-limit full, but it's difficult to make write request block until memory enough.

So I used an independent sstable-store and memory-limiter to limit memory usage

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

If your pull request contains user-facing changes, please specify the types of the changes, and create a release note. Otherwise, please feel free to remove this section.

Types of user-facing changes

Please keep the types that apply to your changes, and remove those that do not apply.

Installation and deployment
Connector (sources & sinks)
SQL commands, functions, and operators
RisingWave cluster configuration changes
Other (please specify in the release note below)

Release note

Please create a release note for your changes. In the release note, focus on the impact on users, and mention the environment or conditions where the impact may occur.

Refer to a related PR or issue link (optional)

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

codecov · 2022-08-11T12:49:09Z

Codecov Report

Merging #4590 (fdb328a) into main (0d920b4) will increase coverage by 0.01%.
The diff coverage is 74.09%.

@@            Coverage Diff             @@
##             main    #4590      +/-   ##
==========================================
+ Coverage   74.05%   74.06%   +0.01%     
==========================================
  Files         862      863       +1     
  Lines      130243   130524     +281     
==========================================
+ Hits        96448    96671     +223     
- Misses      33795    33853      +58

Flag	Coverage Δ
rust	`74.06% <74.09%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/compute/src/server.rs	`0.00% <0.00%> (ø)`
src/storage/compactor/src/server.rs	`0.00% <0.00%> (ø)`
src/storage/src/hummock/compactor/context.rs	`0.00% <0.00%> (ø)`
src/storage/src/hummock/iterator/backward_user.rs	`96.78% <ø> (-0.01%)`	⬇️
src/storage/src/hummock/iterator/mod.rs	`59.78% <ø> (ø)`
src/storage/src/hummock/mod.rs	`85.58% <ø> (ø)`
src/storage/src/hummock/test_utils.rs	`97.61% <ø> (ø)`
src/storage/src/lib.rs	`100.00% <ø> (ø)`
src/storage/src/monitor/state_store_metrics.rs	`84.14% <0.00%> (+0.42%)`	⬆️
src/storage/src/storage_failpoints/test_sstable.rs	`96.55% <ø> (ø)`
... and 21 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace · 2022-08-12T09:25:38Z

This PR could reduce half of memory but it can not still explain why our system use much more memory than statistic.

main branch:

current branch:

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

hzxa21

Great improvement! Thanks for the PR.

Left some comments in advance before I finished the review.

src/compute/src/server.rs

src/storage/compactor/src/server.rs

src/storage/src/hummock/utils.rs

fuyufjh · 2022-08-15T06:13:11Z

Question: is it possible to eliminate caching on the compactor? I don't think caching is helpful here, because every SST being read will be compacted into another SST and the previous files won't be read (compacted) any more.

Little-Wallace · 2022-08-15T06:36:09Z

Question: is it possible to eliminate caching on the compactor? I don't think caching is helpful here, because every SST being read will be compacted into another SST and the previous files won't be read (compacted) any more.

Yes. I would not cache any data in compactor any more in this PR.
I only cache meta-file during compaction job just for several sub-compaction job share the same meta-file.
I do not use LRU-Cache for data file and each sub-compaction just read their own data, which may be some range of a data file.

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

hzxa21

Rest LGTM.

src/compute/src/server.rs

src/storage/compactor/src/server.rs

src/storage/src/hummock/compactor/iterator.rs

wenym1

Rest LGTM

src/storage/src/hummock/utils.rs

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace · 2022-08-16T08:43:41Z

@hzxa21 please take a look again

wenym1

Rest LGTM

src/storage/src/hummock/sstable/multi_builder.rs

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

hzxa21

LGTM

StrikeW · 2022-08-25T05:33:38Z

src/storage/src/hummock/sstable_store.rs

+}
+
+#[async_trait::async_trait]
+pub trait SstableStoreWrite: Send + Sync {


What's the purpose of this trait? Only one implementation, the SstableStore, for it.

Little-Wallace added 4 commits August 11, 2022 14:53

limit memory

088b90d

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

tmp commit

69ab36b

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

separate compaction

6c97395

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

remove data cache

baf66e7

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace requested review from hzxa21 and wenym1 August 11, 2022 09:59

Little-Wallace mentioned this pull request Aug 11, 2022

storage: high memory usage of compactor #3921

Closed

Little-Wallace added 2 commits August 11, 2022 19:16

fix format

38076f4

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

fix test

d4d4bce

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace added 4 commits August 12, 2022 09:58

fix license

99c45bc

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

fix sstable

8289d04

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

do not compress

a6103f7

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

fix memory

6e644d8

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace force-pushed the memory-limit branch from 80ea854 to 6e644d8 Compare August 12, 2022 09:30

Little-Wallace marked this pull request as ready for review August 12, 2022 09:30

Little-Wallace added 2 commits August 12, 2022 17:34

fix test

6554849

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Merge branch 'main' into memory-limit

505646c

hzxa21 reviewed Aug 12, 2022

View reviewed changes

src/compute/src/server.rs Outdated Show resolved Hide resolved

src/storage/compactor/src/server.rs Outdated Show resolved Hide resolved

src/storage/src/hummock/utils.rs Outdated Show resolved Hide resolved

Merge branch 'main' into memory-limit

7a4e9e3

fix format

86cf71e

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

Little-Wallace force-pushed the memory-limit branch from fa4f781 to 86cf71e Compare August 15, 2022 09:02

hzxa21 reviewed Aug 15, 2022

View reviewed changes

src/compute/src/server.rs Outdated Show resolved Hide resolved

src/compute/src/server.rs Outdated Show resolved Hide resolved

src/storage/compactor/src/server.rs Show resolved Hide resolved

src/storage/src/hummock/compactor/iterator.rs Show resolved Hide resolved

wenym1 approved these changes Aug 16, 2022

View reviewed changes

src/storage/src/hummock/utils.rs Outdated Show resolved Hide resolved

This was referenced Aug 16, 2022

refactor: replace Bytes with Vec<u8> for object store to avoid buffer copy when decoding block without compression #4565

Closed

feat(sstable_store): implement streaming upload for sstable store #4644

Merged

Merge branch 'main' into memory-limit

c431498

fix conflict

f539d69

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

wenym1 reviewed Aug 16, 2022

View reviewed changes

src/storage/src/hummock/sstable/multi_builder.rs Outdated Show resolved Hide resolved

add debug log

2fad74b

Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>

hzxa21 approved these changes Aug 17, 2022

View reviewed changes

Little-Wallace added the mergify/can-merge label Aug 17, 2022

Merge branch 'main' into memory-limit

fdb328a

mergify bot merged commit 8883b35 into risingwavelabs:main Aug 17, 2022

hzxa21 mentioned this pull request Aug 18, 2022

feat(storage): Implement BlockStream, SstableStore::get_block_stream(), ObjectStore::streaming_read(), and SstableStreamIterator. #4111

Merged

3 tasks

Little-Wallace deleted the memory-limit branch August 19, 2022 07:28

StrikeW reviewed Aug 25, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(compaction): limit memory usage for compaction read #4590

feat(compaction): limit memory usage for compaction read #4590

Little-Wallace commented Aug 11, 2022 •

edited

Loading

codecov bot commented Aug 11, 2022 •

edited

Loading

Little-Wallace commented Aug 12, 2022

hzxa21 left a comment

fuyufjh commented Aug 15, 2022 •

edited

Loading

Little-Wallace commented Aug 15, 2022 •

edited

Loading

hzxa21 left a comment

wenym1 left a comment

Little-Wallace commented Aug 16, 2022

wenym1 left a comment

hzxa21 left a comment

StrikeW Aug 25, 2022

feat(compaction): limit memory usage for compaction read #4590

feat(compaction): limit memory usage for compaction read #4590

Conversation

Little-Wallace commented Aug 11, 2022 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Types of user-facing changes

Release note

Refer to a related PR or issue link (optional)

codecov bot commented Aug 11, 2022 • edited Loading

Codecov Report

Little-Wallace commented Aug 12, 2022

hzxa21 left a comment

Choose a reason for hiding this comment

fuyufjh commented Aug 15, 2022 • edited Loading

Little-Wallace commented Aug 15, 2022 • edited Loading

hzxa21 left a comment

Choose a reason for hiding this comment

wenym1 left a comment

Choose a reason for hiding this comment

Little-Wallace commented Aug 16, 2022

wenym1 left a comment

Choose a reason for hiding this comment

hzxa21 left a comment

Choose a reason for hiding this comment

StrikeW Aug 25, 2022

Choose a reason for hiding this comment

Little-Wallace commented Aug 11, 2022 •

edited

Loading

codecov bot commented Aug 11, 2022 •

edited

Loading

fuyufjh commented Aug 15, 2022 •

edited

Loading

Little-Wallace commented Aug 15, 2022 •

edited

Loading