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.

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

Db namespaces changes. #2062

Closed
wants to merge 7 commits into from
Closed

Db namespaces changes. #2062

wants to merge 7 commits into from

Conversation

web3-developer
Copy link
Contributor

@web3-developer web3-developer commented Mar 7, 2024

Changes in this PR:

  • Removed the unused and unneeded db backups feature from the RocksStoreRef and other locations.
  • Removed the usage of the RocksDb readonly option from the RocksStoreRef as it complicates the API and was unused. Also the read only RocksDb instances have some issues to be aware of. Basically when starting a read only db connection the data read is from the point in time when the connection was opened but new updates won't be reflected in that connection. See here: https://github.com/facebook/rocksdb/wiki/Read-only-and-Secondary-instances
  • Refactored and created KvStore types for both creating a RocksDb connection and for connecting to a specific RocksDb column family (namespace).
  • New KvStore types have been added into the LegacyPersistantDb backend so that default operations go to the default column family and operations that select a namespace will go to that specific column family.

I'm using the DbNamespace enum as the type which is passed around to decide which namespace to use. The kvt and newKvt procs now have an additional required namespace parameter. I opted to do it this way so that the compiler would help me apply the correct changes in all effected locations in the code. Unfortunately this change touches a lot of places and I couldn't think of a better way to go about it.

The new namespace change currently only affects the legacy persistent mode which is using RocksDB.

Note, that due to the legacy namespace hack/kludge which uses a special prefix byte to virtually separate the various types of data, there will be some difficultly in in removing this tech dept. Many of the tests are based on dumps of the database which are dependent on the data containing these exact prefix bytes so trying to move away from using these prefix bytes will break the tests and would potentially require significant updates to test data.

Also, only the persistent mode has been updated to use the column families to separate the data. This means that most of the tests which use the in memory db modes won't be covering or testing that this column families change will work in production. For this reason I'm wondering if I should also implement some form of separation for the in memory db as well. Alternatively we could opt to only implement column families for the Aristo persistant and Aristo in memory backends and simply leave the legacy dbs as they are because we are planning to switch to Aristo.

@web3-developer web3-developer requested a review from mjfh March 7, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant