Feature: Allow users to tinker with RocksDB configuration via file; Upgrade RocksDb #1638

kwek20 · 2019-10-07T18:10:56Z

Description

In order to easily change rocksdb settings, you can now configure it directly though a config file.
Current settings but in a config would look like this:

create_if_missing=true
create_missing_column_families=true
max_log_file_size=1048576
max_manifest_file_size=1048576
max_open_files=10000
max_background_compactions=1
allow_concurrent_memtable_write=true
max_subcompactions=[your_number_of_cpus]

A full list of options can be found here, but only those inside the DBOptions section will be used: https://github.com/facebook/rocksdb/blob/master/examples/rocksdb_option_file_example.ini
This file CAN be downloaded and used, or one can copy the OPTIONS-0000XX file inside the mainnetdb folder, and rename it to the specified configuration file name.

Configurable options for enabling this are the following:
DB_CONFIG_FILE / --db-config-file: The location of the RocksDB configuration file. By default set to rocksdb-config.properties, which leads to a properties file in the main IRI directory(so not inside mainnet/db folder)

Fixes #1630
Fixes #1629
Fixes #1624

Type of change

Enhancement (a non-breaking change which adds functionality)

How Has This Been Tested?

It loads the config

Checklist:

My code follows the style guidelines for this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

…ocksdb-config

kwek20 · 2019-11-04T16:26:37Z

Splitting up the methods indicated by codacy will in my opinion make it less readable.
Therefor i chose to ignore the suggestion, and this PR is ready for merge!

GalRogozinski

Nice,
Just questions and nits

GalRogozinski · 2019-11-05T12:45:07Z

src/main/java/com/iota/iri/MainInjectionConfiguration.java

@@ -82,6 +83,7 @@ SpentAddressesProvider provideSpentAddressesProvider() {
        PersistenceProvider persistenceProvider = new RocksDBPersistenceProvider(
                configuration.getSpentAddressesDbPath(),
                configuration.getSpentAddressesDbLogPath(),
+                configuration.getDbConfigFile(),


So mainnet-db and spent-addresses-db uses the same name?
IIUC it is just the name but there are still separate locations

Yea same config. i dont really think there will be added value of splitting the 2 apart, unless we hire a rocksdb expert who can tinker enough to make a difference. But assuming we will work on a new persistence layer anyways, i doubt it will happen soon :)

GalRogozinski · 2019-11-05T13:11:06Z

src/main/java/com/iota/iri/storage/rocksDB/RocksDBPersistenceProvider.java

-
-            BlockBasedTableConfig blockBasedTableConfig = new BlockBasedTableConfig().setFilter(bloomFilter);
+            cache = new LRUCache(cacheSize * SizeUnit.KB, 2);
+            compressedCache = new LRUCache(32 * SizeUnit.KB, 10);


Those changes resulted in performance benefits? Or was it something you had to do due to RocksDb upgrade

Due to the update, i did benchmark and unfortunately no notable improvements :(

GalRogozinski · 2019-11-05T13:19:42Z

src/main/java/com/iota/iri/storage/rocksDB/RocksDBPersistenceProvider.java

+     * 
+     * @param dbPath The location where the database will be stored
+     * @param logPath The location where the log files will be stored
+     * @param configFile The location where the RocksDB config is read from


Suggested change

* @param configFile The location where the RocksDB config is read from

* @param configPath The location where the RocksDB config is read from

GalRogozinski · 2019-11-05T13:25:26Z

src/main/java/com/iota/iri/storage/rocksDB/RocksDBPersistenceProvider.java

+                InputStream stream = new FileInputStream(config);
+                try {
+                    configProperties.load(stream);
+                    options = DBOptions.getDBOptionsFromProps(configProperties);
+                } catch (IllegalArgumentException e) {
+                    log.warn("RocksDB configuration file is empty, falling back to default values");
+                } finally {
+                    stream.close();
+                }


I always like to use "try with resources" e.g.
try (InputStream stream = new FileInputStream(config))

Just a bit less verbose. What you did works as well, but I'd rather we use the shorter way throughout the project.

GalRogozinski · 2019-11-06T22:16:32Z

Codacy is still complaining but this is due to existing code you copy pasted, so it is fine

GalRogozinski · 2020-01-19T15:53:34Z

src/main/java/com/iota/iri/storage/rocksDB/RocksDBPersistenceProvider.java

+        if (!BaseIotaConfig.Defaults.DB_LOG_PATH.equals(logPath) && logPath != null) {
+            if (!options.dbLogDir().equals("")) {
+                log.warn("Defined a db log path in config and commandline; Using the command line setting."); 
+            }
+
+            options.setDbLogDir(logPath);
+        }


This is a bit late, I just noticed this code while merging conflicts...
Why is this here @kwek20 ?

This is here due to the fact we can define the db log on the config file and the logbath on iri.ini/cmdline. In order to warwn the user they defined it twice, we add a log

Brord van Wierst added 9 commits September 30, 2019 17:26

Updated rocksdb to 6.2.2

aa1f0f6

Updated deprecated methods

0e0b227

Changed closing order for columnfamily

dce9ef6

Added configurable config

43d8465

config null check

bf19eaa

Merge branch 'dev' into rocksdb-config

fff6e60

rocksdb peristanceprovider javadoc

edbc8fa

Extracted options file creation

2853325

Merge branch 'dev' into rocksdb-config

2095d03

iotaledger deleted a comment Oct 7, 2019

Brord van Wierst added 2 commits October 10, 2019 15:11

Merge branch 'dev' into rocksdb-config

40ced48

Merge branch 'rocksdb-config' of https://github.com/kwek20/iri into r…

6df88b4

…ocksdb-config

kwek20 changed the title ~~Rocksdb config~~ Feature: Rocksdb from a config Oct 30, 2019

Brord van Wierst added 2 commits October 30, 2019 01:22

Merge branch 'dev' into rocksdb-config

77354a6

Merge branch 'dev' into rocksdb-config

0bf94dd

kwek20 requested review from GalRogozinski and luca-moser and removed request for GalRogozinski November 4, 2019 16:26

Merge branch 'dev' into rocksdb-config

0c7cf30

GalRogozinski reviewed Nov 5, 2019

View reviewed changes

try/catch and javadoc fix

cd10e84

This was referenced Nov 6, 2019

Make RocksDb free space upon deleting data #967

Open

Feature: Db size in bytes #1653

Merged

GalRogozinski approved these changes Nov 6, 2019

View reviewed changes

GalRogozinski changed the title ~~Feature: Rocksdb from a config~~ Feature: Allow users to tinker with RocksDB configuration via file; Upgrade RocksDb Nov 6, 2019

GalRogozinski merged commit 621d916 into iotaledger:dev Nov 6, 2019

GalRogozinski mentioned this pull request Jan 6, 2020

Prepare IRI release v.1.9.0 #1703

Open

GalRogozinski reviewed Jan 19, 2020

View reviewed changes

kwek20 mentioned this pull request Feb 12, 2020

Allow users to manually tweak RocksDb Options #1624

Closed

GalRogozinski mentioned this pull request May 6, 2020

Release 1.8.6 #1851

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Allow users to tinker with RocksDB configuration via file; Upgrade RocksDb #1638

Feature: Allow users to tinker with RocksDB configuration via file; Upgrade RocksDb #1638

kwek20 commented Oct 7, 2019 •

edited

Loading

kwek20 commented Nov 4, 2019

GalRogozinski left a comment

GalRogozinski Nov 5, 2019

kwek20 Nov 5, 2019

GalRogozinski Nov 5, 2019

kwek20 Nov 5, 2019

GalRogozinski Nov 5, 2019

GalRogozinski Nov 5, 2019

GalRogozinski commented Nov 6, 2019

GalRogozinski Jan 19, 2020

kwek20 Jan 20, 2020

	* @param configFile The location where the RocksDB config is read from
	* @param configPath The location where the RocksDB config is read from

Feature: Allow users to tinker with RocksDB configuration via file; Upgrade RocksDb #1638

Feature: Allow users to tinker with RocksDB configuration via file; Upgrade RocksDb #1638

Conversation

kwek20 commented Oct 7, 2019 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

kwek20 commented Nov 4, 2019

GalRogozinski 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

GalRogozinski commented Nov 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwek20 commented Oct 7, 2019 •

edited

Loading