Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Add option to disable rocksdb compaction #9011

Merged
merged 1 commit into from
Mar 23, 2020

Conversation

sakridge
Copy link
Contributor

Problem

Crashes with rocksdb compaction.
#9009

Summary of Changes

Add option to disable manual compaction.

Fixes #

@sakridge sakridge changed the title Add option to disable rocks compaction Add option to disable rocksdb compaction Mar 22, 2020
@sakridge sakridge added the CI Pull Request is ready to enter CI label Mar 22, 2020
@brianlong
Copy link
Contributor

This does make sense to me. Currently, compaction is being called within the Rust code AND automatically within RocksDB. Don't need to compact twice. Are you disabling the automated compaction too with this option?

Let me know when you are ready and I can try this branch on my TdS node.

@sakridge sakridge force-pushed the disable-compaction-option branch from 7a58e30 to d5bf863 Compare March 22, 2020 21:33
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Mar 22, 2020
@sakridge sakridge force-pushed the disable-compaction-option branch 3 times, most recently from 0a7f000 to a735c62 Compare March 22, 2020 21:43
@sakridge
Copy link
Contributor Author

This does make sense to me. Currently, compaction is being called within the Rust code AND automatically within RocksDB. Don't need to compact twice. Are you disabling the automated compaction too with this option?

Let me know when you are ready and I can try this branch on my TdS node.

That's right, we have both manual and automatic compaction going with the default settings. This flag would just turn off the manual compaction, I'm hoping the automatic compaction is better tested. It seems less aggressive though, so the user would probably need more free disk space.

@sakridge sakridge requested review from carllin and mvines March 22, 2020 22:38
@mvines mvines added the v1.0 label Mar 22, 2020
validator/src/main.rs Outdated Show resolved Hide resolved
carllin
carllin previously approved these changes Mar 23, 2020
@sakridge sakridge force-pushed the disable-compaction-option branch from a735c62 to 3ab08e5 Compare March 23, 2020 01:25
@mergify mergify bot dismissed carllin’s stale review March 23, 2020 01:26

Pull request has been modified.

@sakridge sakridge force-pushed the disable-compaction-option branch from 3ab08e5 to d569cdd Compare March 23, 2020 03:54
mvines
mvines previously approved these changes Mar 23, 2020
Copy link
Contributor

@mvines mvines left a comment

Choose a reason for hiding this comment

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

One more nit. Rename no_rocks_db_compaction -> no_rocksdb_compaction, and no-rocks-db-compaction -> no-rocksdb-compaction? We don't say rocks db, we say rocksdb.

@sakridge sakridge force-pushed the disable-compaction-option branch from d569cdd to 23a9064 Compare March 23, 2020 04:57
@sakridge sakridge force-pushed the disable-compaction-option branch from 23a9064 to c0803a8 Compare March 23, 2020 04:58
@mergify mergify bot dismissed mvines’s stale review March 23, 2020 04:58

Pull request has been modified.

@sakridge
Copy link
Contributor Author

One more nit. Rename no_rocks_db_compaction -> no_rocksdb_compaction, and no-rocks-db-compaction -> no-rocksdb-compaction? We don't say rocks db, we say rocksdb.

Ok, changed to that.

@sakridge sakridge merged commit 4d2b83d into solana-labs:master Mar 23, 2020
@sakridge sakridge deleted the disable-compaction-option branch March 23, 2020 15:42
mvines pushed a commit to mvines/solana that referenced this pull request Mar 23, 2020
@mvines mvines mentioned this pull request Mar 23, 2020
mvines pushed a commit to mvines/solana that referenced this pull request Mar 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants