feat: database garbage collection #2638

hanabi1224 · 2023-03-08T12:54:18Z

Summary of changes

Changes introduced in this pull request:

Implement a DB wrapper as a drop-in replacement that can be garbage collected.
Hook the DB wrapper into ChainStore
Implement a garbage collection fn that can be scheduled in background task
Trigger GC task with forest-cli

(Blocked by #2635)

Reference issue to close (if applicable)

Closes
#2292
#1708

Change checklist

I have performed a self-review of my own code,
I have made corresponding changes to the documentation,
I have added tests that prove my fix is effective or that my feature works (if possible),
I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

forest/daemon/src/daemon.rs

ipld/src/util.rs

node/db/src/rolling/impls.rs

node/db/src/rolling/index.rs

node/db/src/rolling/mod.rs

node/db/src/rolling/impls.rs

lemmih · 2023-03-17T14:45:00Z

Ran benchmarks with the entire snapshot in the "old" partition and it doesn't affect performance in any way I can measure.

node/db/src/rolling/gc.rs

lemmih

Alright!

I'll add issues for the following improvements:

Load data directly into the old space rather than the new space.
Show progress during garbage collection (and chain exporting).
Possibly get rid of the Store trait altogether.
Yield during GC to allow Forest to shutdown properly.

LesnyRumcajs

It'd be fantastic to make a blogpost about it :)

LesnyRumcajs · 2023-03-21T08:26:05Z

forest/daemon/src/daemon.rs

+    let db_garbage_collector = {
+        let db = db.clone();
+        let chain_store = chain_store.clone();
+        let get_tipset = move || chain_store.heaviest_tipset().as_ref().clone();
+        Arc::new(DbGarbageCollector::new(db, get_tipset))
+    };
+
+    #[allow(clippy::redundant_async_block)]
+    services.spawn({
+        let db_garbage_collector = db_garbage_collector.clone();
+        async move { db_garbage_collector.collect_loop_passive().await }
+    });
+    #[allow(clippy::redundant_async_block)]
+    services.spawn({
+        let db_garbage_collector = db_garbage_collector.clone();
+        async move { db_garbage_collector.collect_loop_event().await }
+    });


It'd be nice to move it to a separate method. To my understanding, those calls in general need each other.

collect_loop_passive and collect_loop_event work independently, either or both can be disabled

LesnyRumcajs · 2023-03-21T08:27:11Z

forest/daemon/src/daemon.rs

@@ -583,28 +600,28 @@ mod test {

    use super::*;

-    #[tokio::test]
+    #[tokio::test(flavor = "multi_thread")]


Why do we need it?

Snapshot import spawns a task for writing to DB now, so that single thread become insufficient

Why is the single threaded runtime insufficient? Surely it can run multiple tasks in parallel.

The test deadlocked so that I made the change

I think we need to debug why that happened. We shouldn't be doing anything blocking.

Just reverted the multi_threading change and the deadlock is gone
350fd3f

The last time I encountered deadlock with tokio was because there were multiple runtimes. It was a bug. :)

node/db/src/rolling/gc.rs

LesnyRumcajs · 2023-03-21T08:33:02Z

node/db/src/rolling/gc.rs

+//! used to speed up the database write operation
+//!
+//! ## Scheduling
+//! 1. GC is triggered automatically when total DB size is greater than 2x of


It'd be nice to add an actual mainnet example.

Do u mean an example output in log?

Just example numbers with mainnet.

Example numbers added

node/db/src/rolling/gc.rs

LesnyRumcajs · 2023-03-21T08:38:19Z

node/db/src/rolling/gc.rs

+                    .ok_or_else(|| anyhow::anyhow!("Cid {cid} not found in blockstore"))?;
+
+                let pair = (cid, block.clone());
+                // Key size is 32 bytes in paritydb


So, does this work at all with RocksDb? If not, how does Forest behave when RocksDb is used?

All functionalities work with rocksdb, however, I did not test thru the crash resilience story for rocksdb so it might not be able to recover when forest is shutdown improperly. I believe we had crash resilience issue with rocksdb before so I think the potential regression is not introduced by this change

So you tested that the GC works with rocksdb? How long does it take? I'm asking because the key size value is hardcoded here.

The time cost is similar. Key size is used for estimating reachable data size as trigger conditions. However, rocksdb is compressed while paritydb is (mostly) not, the gc trigger interval of rocksdb tends to be longer

LesnyRumcajs · 2023-03-21T08:41:14Z

node/db/src/rolling/mod.rs

+//! This module contains a concurrent, semi-space garbage collector. The garbage
+//! collector is guaranteed to be non-blocking and can be expected to run with a
+//! fixed memory overhead and require disk space proportional to the size of the
+//! reachable graph. For example, if the size of the reachable graph is 100GiB,
+//! expect this garbage collector to use 3x100GiB = 300GiB of storage.


Before merging this, we should update the specs of our DO droplets.

Agreed, 320GB is not sufficient if docker and rust toolchains need to be installed

But we could mount a volume of ~500GB on the fly and symbol link the data folder

LesnyRumcajs · 2023-03-21T08:45:00Z

utils/forest_utils/src/db/mod.rs

+            }
+        }
+        self.put_many_keyed(buffer)?;
+        info!(


Is this instead a debug? Also, I'm not sure calling timers by default makes sense in a performance-related method.

This method is called either during snapshot import or during database garbage collection which take 1-2h on laptop or 3-5h on DO droplet. The info log should never flood the screen but it's quite useful for collecting metrics. I would prefer to keep it info level so that it's available in the log by default. What do u think?

If it's helpful, perhaps let's have a proper Prometheus metric? Like, average speed or something like this.

In this case, I think log of individual runs is more helpful than aggregated metrics to understand details and nature of the database and for troubleshooting. How about keeping it as it is, given it only emits 2-3 lines every 1.5-2 days?

utils/forest_utils/src/db/mod.rs

Co-authored-by: Hubert <[email protected]>

hanabi1224 added 11 commits March 8, 2023 17:53

feat: database garbage collection

a50a0d3

rolling db impl and tests

d270673

Merge remote-tracking branch 'origin/main' into hm/db-gc

f07e305

fix lints

da3ce6a

ri FileBacked change

621c11f

Merge remote-tracking branch 'origin/main' into hm/db-gc

ae02c58

remove &mut self from pub APIs

e87c67c

hook up proxy_db

baf8d07

background gc task

0a6f642

cli command and CI

1a40602

fix gc event

ac96d4e

lemmih reviewed Mar 9, 2023

View reviewed changes

forest/daemon/src/daemon.rs Outdated Show resolved Hide resolved

lemmih reviewed Mar 9, 2023

View reviewed changes

ipld/src/util.rs Outdated Show resolved Hide resolved

lemmih reviewed Mar 9, 2023

View reviewed changes

node/db/src/rolling/impls.rs Outdated Show resolved Hide resolved

lemmih reviewed Mar 9, 2023

View reviewed changes

node/db/src/rolling/impls.rs Outdated Show resolved Hide resolved

lemmih reviewed Mar 9, 2023

View reviewed changes

node/db/src/rolling/index.rs Outdated Show resolved Hide resolved

lemmih reviewed Mar 9, 2023

View reviewed changes

node/db/src/rolling/mod.rs Outdated Show resolved Hide resolved

hanabi1224 added 6 commits March 9, 2023 21:43

Merge branch 'main' into hm/db-gc

5fc9f51

buffered copy during GC

9d8eb59

Merge branch 'main' into hm/db-gc

08b0d66

resolve review comments

ee240af

Merge remote-tracking branch 'origin/main' into hm/db-gc

8bbf6db

aggregated error

f89a7a7

hanabi1224 marked this pull request as ready for review March 9, 2023 16:09

hanabi1224 requested review from elmattic, LesnyRumcajs, jdjaustin, creativcoder and sudo-shashank as code owners March 9, 2023 16:09

lemmih reviewed Mar 9, 2023

View reviewed changes

node/db/src/rolling/impls.rs Outdated Show resolved Hide resolved

hanabi1224 force-pushed the hm/db-gc branch from 3c72222 to 8fca01f Compare March 17, 2023 10:58

Merge branch 'main' into hm/db-gc

47be513

lemmih reviewed Mar 17, 2023

View reviewed changes

node/db/src/rolling/gc.rs Show resolved Hide resolved

hanabi1224 added 4 commits March 17, 2023 22:58

Merge branch 'main' into hm/db-gc

1ce7265

Merge remote-tracking branch 'origin/main' into hm/db-gc

3ed8005

simplify open_db

b7d4c8d

change db_gc rpc access to write

ee30872

lemmih approved these changes Mar 20, 2023

View reviewed changes

lemmih mentioned this pull request Mar 20, 2023

Improve GC performance when loading a snapshot #2686

Closed

Merge branch 'main' into hm/db-gc

c341894

LesnyRumcajs reviewed Mar 21, 2023

View reviewed changes

hanabi1224 and others added 7 commits March 21, 2023 18:18

Apply suggestions from code review

6896b53

Co-authored-by: Hubert <[email protected]>

Merge remote-tracking branch 'origin/main' into hm/db-gc

eeaee7e

const DB_KEY_SIZE

a66158d

revert snapshot import tests to using single thread

350fd3f

Merge remote-tracking branch 'origin/main' into hm/db-gc

44e079a

sample numbers

ae67da5

Merge remote-tracking branch 'origin/main' into hm/db-gc

e437e01

LesnyRumcajs approved these changes Mar 21, 2023

View reviewed changes

hanabi1224 added 4 commits March 22, 2023 09:04

Merge remote-tracking branch 'origin/main' into hm/db-gc

220a03c

Merge branch 'main' into hm/db-gc

2586fe8

Merge branch 'main' into hm/db-gc

a42ac9b

FOREST_GC_TRIGGER_FACTOR env var

2d077eb

hanabi1224 merged commit c1674e8 into main Mar 22, 2023

hanabi1224 deleted the hm/db-gc branch March 22, 2023 13:21

This was referenced Mar 23, 2023

fix: fvm_ipld_encoding, forest_db, and ethers dependencies fleek-network/ursa#448

Merged

fix: fvm_ipld_encoding, forest_db, and ethers dependencies fleek-network/ursa#450

Merged

jdjaustin mentioned this pull request Jun 16, 2023

Restore --tipset functionality to forest-cli snapshot export #2999

Closed

aakoshh mentioned this pull request Dec 19, 2023

State tree / app state garbage collection consensus-shipyard/ipc#154

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: database garbage collection #2638

feat: database garbage collection #2638

hanabi1224 commented Mar 8, 2023 •

edited

Loading

lemmih commented Mar 17, 2023

lemmih left a comment

LesnyRumcajs left a comment

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023

lemmih Mar 21, 2023

hanabi1224 Mar 21, 2023

lemmih Mar 21, 2023

hanabi1224 Mar 21, 2023

LesnyRumcajs Mar 21, 2023

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023 •

edited

Loading

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023 •

edited

Loading

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023

hanabi1224 Mar 21, 2023

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023

LesnyRumcajs Mar 21, 2023

hanabi1224 Mar 21, 2023

LesnyRumcajs Mar 21, 2023

feat: database garbage collection #2638

feat: database garbage collection #2638

Conversation

hanabi1224 commented Mar 8, 2023 • edited Loading

Summary of changes

Reference issue to close (if applicable)

Other information and links

Change checklist

lemmih commented Mar 17, 2023

lemmih left a comment

Choose a reason for hiding this comment

LesnyRumcajs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanabi1224 Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanabi1224 Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanabi1224 commented Mar 8, 2023 •

edited

Loading

hanabi1224 Mar 21, 2023 •

edited

Loading

hanabi1224 Mar 21, 2023 •

edited

Loading