Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add new flag for maximum file descriptors in rocksdb. #2386

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add new flag for maximum file descriptors in rocksdb. #2386

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
87a2240
Add new flag for maximum file descriptors in rocksdb.
AurelienFT Oct 23, 2024
ef6a1d0
Merge branch 'master' into limit_rocks_db_fd
AurelienFT Oct 23, 2024
929f0bd
Replace unwrap with expect and change vlaue in tests.
AurelienFT Oct 23, 2024
09d4ddd
Merge branch 'limit_rocks_db_fd' of github.com:FuelLabs/fuel-core int…
AurelienFT Oct 23, 2024
82eaa5f
Update expect message.
AurelienFT Oct 23, 2024
c1bec23
Update default function to helper.
AurelienFT Oct 24, 2024
9d41bd5
Merge branch 'master' into limit_rocks_db_fd
MitchTurner Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/fuel-core/src/cli/rollback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct Command {
#[clap(
long = "rocksdb-max-fds",
env,
default_value = getrlimit(Resource::NOFILE).map(|(_, hard)| i32::try_from(hard.saturating_div(2)).unwrap_or(i32::MAX)).unwrap().to_string()
default_value = getrlimit(Resource::NOFILE).map(|(_, hard)| i32::try_from(hard.saturating_div(2)).unwrap_or(i32::MAX)).expect("Failed to get OS max file descriptors.").to_string()
Copy link
Member

Choose a reason for hiding this comment

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

Oh. Sorry. I mean specify why it shouldn't panic:

We recommend that expect messages are used to describe the reason you expect the Result should be Ok.

As suggested by the docs:
https://doc.rust-lang.org/std/result/enum.Result.html#method.expect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I didn't correctly understood you first message and didn't know about this convention. Happy to learn :D Ty

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh didn't know this. I've definitely done a bunch of .expect("failed to blah blah") 😅 TIL

)]
pub rocksdb_max_fds: i32,

Expand Down
2 changes: 1 addition & 1 deletion bin/fuel-core/src/cli/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub struct Command {
#[clap(
long = "rocksdb-max-fds",
env,
default_value = getrlimit(Resource::NOFILE).map(|(_, hard)| i32::try_from(hard.saturating_div(2)).unwrap_or(i32::MAX)).unwrap().to_string()
default_value = getrlimit(Resource::NOFILE).map(|(_, hard)| i32::try_from(hard.saturating_div(2)).unwrap_or(i32::MAX)).expect("Failed to get OS max file descriptors.").to_string()
)]
pub rocksdb_max_fds: i32,

Expand Down
14 changes: 7 additions & 7 deletions bin/fuel-core/src/cli/snapshot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub struct Command {
#[clap(
long = "rocksdb-max-fds",
env,
default_value = getrlimit(Resource::NOFILE).map(|(_, hard)| i32::try_from(hard.saturating_div(2)).unwrap_or(i32::MAX)).unwrap().to_string()
default_value = getrlimit(Resource::NOFILE).map(|(_, hard)| i32::try_from(hard.saturating_div(2)).unwrap_or(i32::MAX)).expect("Failed to get OS max file descriptors.").to_string()
)]
pub rocksdb_max_fds: i32,

Expand Down Expand Up @@ -690,7 +690,7 @@ mod tests {
std::fs::create_dir(&db_path)?;

let mut db =
DbPopulator::new(open_db(&db_path, None, -1)?, StdRng::seed_from_u64(2));
DbPopulator::new(open_db(&db_path, None, 512)?, StdRng::seed_from_u64(2));
let state = db.given_persisted_data();
db.flush();

Expand All @@ -703,7 +703,7 @@ mod tests {
chain_config: None,
encoding_command: Some(EncodingCommand::Encoding { encoding }),
},
rocksdb_max_fds: -1,
rocksdb_max_fds: 512,
});

// Because the test_case macro doesn't work with async tests
Expand Down Expand Up @@ -744,7 +744,7 @@ mod tests {
let snapshot_dir = temp_dir.path().join("snapshot");
let db_path = temp_dir.path().join("db");
let mut db =
DbPopulator::new(open_db(&db_path, None, -1)?, StdRng::seed_from_u64(2));
DbPopulator::new(open_db(&db_path, None, 512)?, StdRng::seed_from_u64(2));

let state = db.given_persisted_data();
db.flush();
Expand All @@ -763,7 +763,7 @@ mod tests {
},
}),
},
rocksdb_max_fds: -1,
rocksdb_max_fds: 512,
});

tokio::runtime::Runtime::new()
Expand All @@ -789,7 +789,7 @@ mod tests {

let db_path = temp_dir.path().join("db");
let mut db =
DbPopulator::new(open_db(&db_path, None, -1)?, StdRng::seed_from_u64(2));
DbPopulator::new(open_db(&db_path, None, 512)?, StdRng::seed_from_u64(2));

let original_state = db.given_persisted_data().sorted().into_state_config();

Expand All @@ -815,7 +815,7 @@ mod tests {
output_dir: snapshot_dir.clone(),
max_database_cache_size: DEFAULT_DATABASE_CACHE_SIZE,
subcommand: SubCommands::Contract { contract_id },
rocksdb_max_fds: -1,
rocksdb_max_fds: 512,
})
.await?;

Expand Down
2 changes: 1 addition & 1 deletion crates/fuel-core/src/database.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ mod tests {
temp_dir.path(),
1024 * 1024 * 1024,
Default::default(),
-1,
512,
)
.unwrap();
// rocks db fails
Expand Down
2 changes: 1 addition & 1 deletion crates/fuel-core/src/service/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl Config {
state_rewind_policy:
crate::state::historical_rocksdb::StateRewindPolicy::RewindFullRange,
#[cfg(feature = "rocksdb")]
max_fds: -1,
max_fds: 512,
};
let starting_gas_price = 0;
let gas_price_change_percent = 0;
Expand Down
19 changes: 8 additions & 11 deletions crates/fuel-core/src/state/rocks_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ where
path,
enum_iterator::all::<Description::Column>().collect::<Vec<_>>(),
capacity,
-1,
512,
);
let mut db = result?;

Expand Down Expand Up @@ -290,9 +290,6 @@ where
opts.set_bytes_per_sync(1048576);
opts.set_max_open_files(max_fds);

#[cfg(feature = "test-helpers")]
opts.set_max_open_files(512);

let existing_column_families = DB::list_cf(&opts, &path).unwrap_or_default();

let mut cf_descriptors_to_open = BTreeMap::new();
Expand Down Expand Up @@ -890,7 +887,7 @@ mod tests {
fn create_db() -> (RocksDb<OnChain>, TempDir) {
let tmp_dir = TempDir::new().unwrap();
(
RocksDb::default_open(tmp_dir.path(), None, -1).unwrap(),
RocksDb::default_open(tmp_dir.path(), None, 512).unwrap(),
tmp_dir,
)
}
Expand All @@ -903,7 +900,7 @@ mod tests {
let old_columns =
vec![Column::Coins, Column::Messages, Column::UploadedBytecodes];
let database_with_old_columns =
RocksDb::<OnChain>::open(tmp_dir.path(), old_columns.clone(), None, -1)
RocksDb::<OnChain>::open(tmp_dir.path(), old_columns.clone(), None, 512)
.expect("Failed to open database with old columns");
drop(database_with_old_columns);

Expand All @@ -912,7 +909,7 @@ mod tests {
new_columns.push(Column::ContractsAssets);
new_columns.push(Column::Metadata);
let database_with_new_columns =
RocksDb::<OnChain>::open(tmp_dir.path(), new_columns, None, -1).map(|_| ());
RocksDb::<OnChain>::open(tmp_dir.path(), new_columns, None, 512).map(|_| ());

// Then
assert_eq!(Ok(()), database_with_new_columns);
Expand Down Expand Up @@ -1081,7 +1078,7 @@ mod tests {
// When
let columns = enum_iterator::all::<<OnChain as DatabaseDescription>::Column>()
.collect::<Vec<_>>();
let result = RocksDb::<OnChain>::open(tmp_dir.path(), columns, None, -1);
let result = RocksDb::<OnChain>::open(tmp_dir.path(), columns, None, 512);

// Then
assert!(result.is_err());
Expand All @@ -1100,7 +1097,7 @@ mod tests {
old_columns.clone(),
None,
false,
-1,
512,
)
.map(|_| ());

Expand All @@ -1122,7 +1119,7 @@ mod tests {
secondary_temp.path(),
old_columns.clone(),
None,
-1,
512,
)
.map(|_| ());

Expand Down Expand Up @@ -1215,7 +1212,7 @@ mod tests {
.skip(1)
.collect::<Vec<_>>();
let open_with_part_of_columns =
RocksDb::<OnChain>::open(tmp_dir.path(), part_of_columns, None, -1);
RocksDb::<OnChain>::open(tmp_dir.path(), part_of_columns, None, 512);

// Then
let _ = open_with_part_of_columns
Expand Down
10 changes: 6 additions & 4 deletions tests/tests/health.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ async fn can_restart_node() {
// start node once
{
let database =
Database::open_rocksdb(tmp_dir.path(), None, Default::default(), -1).unwrap();
Database::open_rocksdb(tmp_dir.path(), None, Default::default(), 512)
.unwrap();
let first_startup = FuelService::from_database(database, Config::local_node())
.await
.unwrap();
Expand All @@ -41,7 +42,8 @@ async fn can_restart_node() {

{
let database =
Database::open_rocksdb(tmp_dir.path(), None, Default::default(), -1).unwrap();
Database::open_rocksdb(tmp_dir.path(), None, Default::default(), 512)
.unwrap();
let _second_startup = FuelService::from_database(database, Config::local_node())
.await
.unwrap();
Expand All @@ -56,7 +58,7 @@ async fn can_restart_node_with_transactions() {
{
// Given
let database =
CombinedDatabase::open(tmp_dir.path(), capacity, Default::default(), -1)
CombinedDatabase::open(tmp_dir.path(), capacity, Default::default(), 512)
.unwrap();
let service = FuelService::from_combined_database(database, Config::local_node())
.await
Expand All @@ -75,7 +77,7 @@ async fn can_restart_node_with_transactions() {
{
// When
let database =
CombinedDatabase::open(tmp_dir.path(), capacity, Default::default(), -1)
CombinedDatabase::open(tmp_dir.path(), capacity, Default::default(), 512)
.unwrap();
let service = FuelService::from_combined_database(database, Config::local_node())
.await
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/poa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ async fn can_get_sealed_block_from_poa_produced_block_when_signing_with_kms() {

// stop the node and just grab the database
let db_path = driver.kill().await;
let db =
CombinedDatabase::open(db_path.path(), 1024 * 1024, Default::default()).unwrap();
let db = CombinedDatabase::open(db_path.path(), 1024 * 1024, Default::default(), 512)
.unwrap();

let view = db.on_chain().latest_view().unwrap();

Expand Down
4 changes: 2 additions & 2 deletions tests/tests/relayer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ async fn can_restart_node_with_relayer_data() {
{
// Given
let database =
CombinedDatabase::open(tmp_dir.path(), capacity, Default::default(), -1)
CombinedDatabase::open(tmp_dir.path(), capacity, Default::default(), 512)
.unwrap();

let service = FuelService::from_combined_database(database, config.clone())
Expand All @@ -351,7 +351,7 @@ async fn can_restart_node_with_relayer_data() {
{
// When
let database =
CombinedDatabase::open(tmp_dir.path(), capacity, Default::default(), -1)
CombinedDatabase::open(tmp_dir.path(), capacity, Default::default(), 512)
.unwrap();
let service = FuelService::from_combined_database(database, config)
.await
Expand Down
Loading