Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

RocksDB - Renaming / creation of some parameters and change of default value for create_if_missing #9692

Merged
merged 3 commits into from
Nov 19, 2020

Conversation

victorj8
Copy link
Contributor

@victorj8 victorj8 commented Nov 18, 2020

Change Description

Changes to RocksDB parameter names:

  • rocksdb-threads changed to persistent-storage-num-threads
  • rocksdb-files changed to persistent-storage-max-num-files
  • rocksdb-write-buffer-size-mb changed to persistent-storage-write-buffer-size-mb

New RocksDB parameter:

  • persistent-storage-bytes-per-sync (used to control the write rate of flushes and compactions)

Other

  • create_if_missing is now set to true.

Change Type

Select ONE

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

We need to change the documentation regarding the new parameter names and the addition of the new parameter.

Copy link

@gssagoo gssagoo left a comment

Choose a reason for hiding this comment

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

I have left comments in line to remove and update some of the comments accompanying the changes we have made.

libraries/chain/include/eosio/chain/config.hpp Outdated Show resolved Hide resolved
Comment on lines +313 to +314
("persistent-storage-bytes-per-sync", bpo::value<uint64_t>()->default_value(config::default_persistent_storage_bytes_per_sync),
"Rocksdb write rate of flushes and compactions.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want its unit to be in MiB, but then it might be too coarse. If we do use MiB, here is a tricky one: you'll need to divide config::default_persistent_storage_bytes_per_sync by 1024 * 1024 here for the default value and then multiply back below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was told that this value may be getting values such as 512K, that's why i didn't use the MiB here.

@victorj8 victorj8 merged commit 49ce128 into develop Nov 19, 2020

// Use this option to increase the number of threads
// used to open the files.
options.max_file_opening_threads = cfg.rocksdb_threads; // Default should be the # of Cores
options.max_file_opening_threads = cfg.persistent_storage_num_threads;

// Write Buffer Size - Sets the size of a single
// memtable. Once memtable exceeds this size, it is
// marked immutable and a new one is created.
// Default should be 128MB
Copy link

Choose a reason for hiding this comment

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

@victorj8 Apologies - i missed this yesterday, but can you remove the comment '// Default should be 128MB'. thanks

Copy link

@gssagoo gssagoo left a comment

Choose a reason for hiding this comment

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

All looks good besides that comment cleanup that i commented on which is not a show stopper and can be addressed afterwards.

@heifner heifner deleted the rocksdb-params-work branch June 10, 2021 16:56
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.

3 participants