Take storage configuration into consideration when opening storage #6857
Comments
mina86
added a commit
that referenced
this issue
May 21, 2022
Introduce near_store::StoreOpener as a new interface for opening storage. This gets rid of create_store and create_store_with_config functions and converts all users of those into users of StoreOpener. While doing that, convert a few uses of create_store (i.e. calls which ignore StoreConfig) into ones where the configuration is taken into consideration. Issue: #6857
near-bulldozer bot
pushed a commit
that referenced
this issue
May 24, 2022
Introduce near_store::StoreOpener as a new interface for opening storage. This gets rid of create_store and create_store_with_config functions and converts all users of those into users of StoreOpener. While doing that, convert a few uses of create_store (i.e. calls which ignore StoreConfig) into ones where the configuration is taken into consideration. Issue: #6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
May 27, 2022
RuntimeTestbed doesn’t care where the database is stored so changes how the database is opened inside StateDump::from_dir doesn’t really matter. Still, this change doesn’t actually affect the behaviour of the testbed code because with StoreOpener::home instead of StoreOpener::path being called, StoreOpener::open constructs path to the database the same way get_store_path does so resulting path is the same in old and new code. Issue: near#6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
May 29, 2022
Use StoreOpener’s encapsulation of the details regarding path to the RocksDB database. To make this work deprecate `get_store_version` method (which takes path to the database as argument) in favour of a new `StoreOpener::get_version_if_exists` method which, as name implies, returns version of the database if it exists. Issue: near#6857
near-bulldozer bot
pushed a commit
that referenced
this issue
May 31, 2022
…6891) RuntimeTestbed doesn’t care where the database is stored so changes how the database is opened inside StateDump::from_dir doesn’t really matter. Still, this change doesn’t actually affect the behaviour of the testbed code because with StoreOpener::home instead of StoreOpener::path being called, StoreOpener::open constructs path to the database the same way get_store_path does so resulting path is the same in old and new code. Issue: #6857
near-bulldozer bot
pushed a commit
that referenced
this issue
May 31, 2022
Use StoreOpener’s encapsulation of the details regarding path to the RocksDB database. To make this work deprecate `get_store_version` method (which takes path to the database as argument) in favour of a new `StoreOpener::get_version_if_exists` method which, as name implies, returns version of the database if it exists. Issue: #6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 1, 2022
This allows removal of get_store_version (since the code is now using StoreOpener::get_version_if_exists is used) and reduces uses of the StoreOpener::path method. Issue: near#6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 1, 2022
This allows removal of get_store_version (since the code is now using StoreOpener::get_version_if_exists is used) and reduces uses of the StoreOpener::path method. Issue: near#6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 1, 2022
This allows removal of get_store_version (since the code is now using StoreOpener::get_version_if_exists is used) and reduces uses of the StoreOpener::path method. Issue: near#6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 1, 2022
This allows removal of get_store_version (since the code is now using StoreOpener::get_version_if_exists is used) and reduces uses of the StoreOpener::path method. Issue: near#6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 1, 2022
This allows removal of get_store_version (since the code is now using StoreOpener::get_version_if_exists is used) and reduces uses of the StoreOpener::path method. Issue: near#6857
|
This can also break setups for example currently I have to patch neard to not bypass my max_open_files limit when checking the rocks db version, because my unittests run a sandbox that limits the amount of open file descriptors. |
|
This shouldn’t affect tests since tests keep using default configuration. In non-tests this will make them respect max_open_files limit set in configuration file. @Mic92, do you have no way to update the hard limit in the sandbox? |
near-bulldozer bot
pushed a commit
that referenced
this issue
Jun 1, 2022
This allows removal of get_store_version (since the code is now using StoreOpener::get_version_if_exists is used) and reduces uses of the StoreOpener::path method. Issue: #6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 1, 2022
Whenever NearConfig is available, use StoreConfig from it rather than assuming default store configuration when creating StoreOpener (and thus opening storage). This will become important when path is added to the configuration. With this change, the only remaining uses of with_default_config are in test code where we would use default store configuration anyway. Issue: near#6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 1, 2022
Add `path` field to StoreConfig which makes it possible to configure where RocksDB is located rather than hard-coding the location to `data` subdirectory. This helps with setup where neard home directory and the storage live on separate file systems. Previously such configurations required using of symbolic links or mounting storage inside neard home. Both of those solutions weren’t as clean as ability to point neard directly at the right place. This will become even more useful with planned cold storage which will call for all archival nodes to use multiple disks for the data (SSD for head data and HDD for archival data). Fixes: near#6857
near-bulldozer bot
pushed a commit
that referenced
this issue
Jun 1, 2022
Whenever NearConfig is available, use StoreConfig from it rather than assuming default store configuration when creating StoreOpener (and thus opening storage). This will become important when path is added to the configuration. With this change, the only remaining uses of with_default_config are in test code where we would use default store configuration anyway. Issue: #6857
nikurt
pushed a commit
that referenced
this issue
Jun 2, 2022
…6891) RuntimeTestbed doesn’t care where the database is stored so changes how the database is opened inside StateDump::from_dir doesn’t really matter. Still, this change doesn’t actually affect the behaviour of the testbed code because with StoreOpener::home instead of StoreOpener::path being called, StoreOpener::open constructs path to the database the same way get_store_path does so resulting path is the same in old and new code. Issue: #6857
nikurt
pushed a commit
that referenced
this issue
Jun 2, 2022
Use StoreOpener’s encapsulation of the details regarding path to the RocksDB database. To make this work deprecate `get_store_version` method (which takes path to the database as argument) in favour of a new `StoreOpener::get_version_if_exists` method which, as name implies, returns version of the database if it exists. Issue: #6857
nikurt
pushed a commit
that referenced
this issue
Jun 2, 2022
This allows removal of get_store_version (since the code is now using StoreOpener::get_version_if_exists is used) and reduces uses of the StoreOpener::path method. Issue: #6857
nikurt
pushed a commit
that referenced
this issue
Jun 2, 2022
Whenever NearConfig is available, use StoreConfig from it rather than assuming default store configuration when creating StoreOpener (and thus opening storage). This will become important when path is added to the configuration. With this change, the only remaining uses of with_default_config are in test code where we would use default store configuration anyway. Issue: #6857
near-bulldozer bot
pushed a commit
that referenced
this issue
Jun 3, 2022
Add `path` field to StoreConfig which makes it possible to configure
where RocksDB is located rather than hard-coding the location to
`data` subdirectory. The directory can be set under `store.path` JSON
path in `config.json` file, e.g.:
```
/* config.json */
{
"genesis_file": "genesis.json",
…
"store": {
"path": "/srv/near-hot-data",
…
},
…
}
```
This helps with setup where neard home directory and the storage live
on separate file systems. Previously such configurations required
using of symbolic links or mounting storage inside neard home. Both
of those solutions weren’t as clean as ability to point neard directly
at the right place.
This will become even more useful with planned cold storage which will
call for all archival nodes to use multiple disks for the data (SSD
for head data and HDD for archival data).
Fixes: #6857
Sorry that came a bit out of context. I was talking about our tests not the one used by neard. We are building a supervisor on top of neard, that implements ha failover and testing them in a nix build.
On the machines we control yes, but we want later also normal nix users to be able to test this when we publish the code without having to change their setup. |
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 4, 2022
Add home_dir as argument to StoreOpener::new and with_default_config methods. All call sites allready pass the home directory so this is a purely refactoring change. With that the home directory is no longer necessary. Issue: near#6857
near-bulldozer bot
pushed a commit
that referenced
this issue
Jun 6, 2022
) Add home_dir as argument to StoreOpener::new and with_default_config methods. All call sites allready pass the home directory so this is a purely refactoring change. With that the home method is no longer necessary. Issue: #6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 6, 2022
Pass StoreConfig from NearConfig to streamer::start so that it can take it into account when opening storage rather than assuming default configuration. Issue: near#6857
near-bulldozer bot
pushed a commit
that referenced
this issue
Jun 7, 2022
Pass StoreConfig from NearConfig to streamer::start so that it can take it into account when opening storage rather than assuming default configuration. Issue: #6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 7, 2022
read_trie_items reads NearConfig but then ignores it when opening a store using default store configuration. Change so that it takes StoreConfig embedded in NearConfig into account. Issue: near#6857
near-bulldozer bot
pushed a commit
that referenced
this issue
Jun 8, 2022
read_trie_items reads NearConfig but then ignores it when opening a store using default store configuration. Change so that it takes StoreConfig embedded in NearConfig into account. Issue: #6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 10, 2022
Firstly, replace StoreOpener constructors with static methods on the Store struct. Specifically, add Store::opener (which acts just like StoreOpener::new which is now private) and Store::tmp_opener (which replaces StoreOpener::with_default_config). Secondly, incorporate creation of the temporary directory into the Store::tmp_opener method. Previously, test code would create a temporary directory and use StoreOpener::with_default_config. Now, this is all rolled into single Store::tmp_opener method. The motivation for that is to make it painfully obvious that Store::tmp_opener is meant for tests only. Anything that operates on a non-temporary storage will have to use Store::opener and provide it with a StoreConfig. Lastly, change StoreOpener to resolve the path to the storage during construction. This replaces `home_dir` that the struct was holding with a `path: PathBuf`. This means that StoreOpener::get_path will no longer resolve the path on each call and have its return value ready to go. Issue: near#6857
mina86
added a commit
that referenced
this issue
Jun 10, 2022
…7015) Firstly, replace StoreOpener constructors with static methods on the Store struct. Specifically, add Store::opener (which acts just like StoreOpener::new which is now private) and Store::tmp_opener (which replaces StoreOpener::with_default_config). Secondly, incorporate creation of the temporary directory into the Store::tmp_opener method. Previously, test code would create a temporary directory and use StoreOpener::with_default_config. Now, this is all rolled into single Store::tmp_opener method. The motivation for that is to make it painfully obvious that Store::tmp_opener is meant for tests only. Anything that operates on a non-temporary storage will have to use Store::opener and provide it with a StoreConfig. Lastly, change StoreOpener to resolve the path to the storage during construction. This replaces `home_dir` that the struct was holding with a `path: PathBuf`. This means that StoreOpener::get_path will no longer resolve the path on each call and have its return value ready to go. Issue: #6857
nikurt
pushed a commit
that referenced
this issue
Jun 13, 2022
…7015) Firstly, replace StoreOpener constructors with static methods on the Store struct. Specifically, add Store::opener (which acts just like StoreOpener::new which is now private) and Store::tmp_opener (which replaces StoreOpener::with_default_config). Secondly, incorporate creation of the temporary directory into the Store::tmp_opener method. Previously, test code would create a temporary directory and use StoreOpener::with_default_config. Now, this is all rolled into single Store::tmp_opener method. The motivation for that is to make it painfully obvious that Store::tmp_opener is meant for tests only. Anything that operates on a non-temporary storage will have to use Store::opener and provide it with a StoreConfig. Lastly, change StoreOpener to resolve the path to the storage during construction. This replaces `home_dir` that the struct was holding with a `path: PathBuf`. This means that StoreOpener::get_path will no longer resolve the path on each call and have its return value ready to go. Issue: #6857
mina86
added a commit
to mina86/nearcore
that referenced
this issue
Jun 13, 2022
The two undocumented `unsafe_reset_all` and `unsafe_reset_data` commands have been deprecated since 1.25. Get rid of them. This also gets rid of `get_store_path` function which is the last place where path to the RocksDB is assumed to be `<neard-home>/data`. Issue: near#6857
near-bulldozer bot
pushed a commit
that referenced
this issue
Jun 14, 2022
) The two undocumented `unsafe_reset_all` and `unsafe_reset_data` commands have been deprecated since 1.25. Get rid of them. This also gets rid of `get_store_path` function which is the last place where path to the RocksDB is assumed to be `<neard-home>/data`. Issue: #6857
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently we have a bunch of places where we open RocksDB with
create_storewhich uses a default store configuration. The defaults usually work just fine so this is not an issue but as we add more options it may be necessary to take them into consideration. In particular, I’m envisioning adding apathtoStoreConfigsuch that it won’t be possible to assume path being<home>/data.The text was updated successfully, but these errors were encountered: