-
Notifications
You must be signed in to change notification settings - Fork 161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --halt-after-import
flag to CLI
#1835
Changes from all commits
4080e47
0330ea1
2510da2
3b8c3db
01fbdd3
b59bce4
c75fd32
4c6a1ba
ebec6f5
f65827f
9bacd9d
512395f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,12 @@ | |
|
||
use super::errors::Error; | ||
use super::Store; | ||
use crate::rocks_config::{compaction_style_from_str, compression_type_from_str, RocksDbConfig}; | ||
use crate::rocks_config::{ | ||
compaction_style_from_str, compression_type_from_str, log_level_from_str, RocksDbConfig, | ||
}; | ||
use cid::Cid; | ||
use fvm_ipld_blockstore::Blockstore; | ||
pub use rocksdb::{Options, WriteBatch, DB}; | ||
pub use rocksdb::{LogLevel, Options, WriteBatch, DB}; | ||
use std::{path::Path, sync::Arc}; | ||
|
||
/// `RocksDB` instance this satisfies the [Store] interface. | ||
|
@@ -40,13 +42,19 @@ impl RocksDb { | |
} | ||
if let Some(compaction_style) = &config.compaction_style { | ||
db_opts.set_compaction_style(compaction_style_from_str(compaction_style).unwrap()); | ||
db_opts.set_disable_auto_compactions(false); | ||
} else { | ||
db_opts.set_disable_auto_compactions(true); | ||
Comment on lines
+45
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This appears to be unrelated to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, this was part of testing to see if RocksDb was being dropped after import. I've left some of the configuration logic in as it was part of @elmattic 's work |
||
} | ||
if let Some(compression_type) = &config.compression_type { | ||
db_opts.set_compression_type(compression_type_from_str(compression_type).unwrap()); | ||
} | ||
if config.enable_statistics { | ||
db_opts.enable_statistics(); | ||
}; | ||
|
||
db_opts.set_log_level(log_level_from_str(&config.log_level).unwrap()); | ||
|
||
Ok(Self { | ||
db: Arc::new(DB::open(&db_opts, path)?), | ||
}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some doc comment help for this new flag. Consider refactoring to use doc comments for the other flags too (this will gives us documentation like IDE tooltip and spell checking for free).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lemmih do you think such self-describing flags need help comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The help message could even add more details such as "Halt with exit code 0 after successfully importing a snapshot".