Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow full and light client on-disk databases to co-exist #6880

Closed
expenses opened this issue Aug 12, 2020 · 11 comments · Fixed by #9645
Closed

Allow full and light client on-disk databases to co-exist #6880

expenses opened this issue Aug 12, 2020 · 11 comments · Fixed by #9645
Labels
U2-some_time_soon Issue is worth doing soon. Z0-trivial Writing the issue is of the same difficulty as patching the code.

Comments

@expenses
Copy link
Contributor

This is a nice small issue to give the light client some love. If you run the full node for a bit, then a light node with --light, you get the following error:

Error: Service(Client(Backend("Unexpected database type. Expected: light")))

It's easy enough to change the paths where the databases are put, it might be best to put the full db in /db/full/ and the light db in /db/light.
@expenses expenses added U2-some_time_soon Issue is worth doing soon. Z0-trivial Writing the issue is of the same difficulty as patching the code. labels Aug 12, 2020
@expenses
Copy link
Contributor Author

It's also important to consider that moving the path of the full db means that you have to add code that migrates from the old path.

@expenses expenses changed the title Allow full and light on-disk databases to co-exist Allow full and light client on-disk databases to co-exist Aug 12, 2020
@expenses expenses mentioned this issue Aug 15, 2020
Merged
@expenses
Copy link
Contributor Author

It's easy enough to change the paths where the databases are put, it might be best to put the full db in /db/full/ and the light db in /db/light.

(Pinging to get a consensus) @seunlanlege @rakanalh @danforbes @gnunicorn I believe this would be the best way to things, anyone else have an opinion?

@rakanalh
Copy link
Contributor

I think this is worth doing as i imagine there would be use cases where one would want to switch between the two.

@seunlanlege
Copy link
Contributor

+1

@niklasad1
Copy link
Member

Sounds good to me, however, duplicate of #2204 but the description is better in this issue so feel free to close #2204

@bkchr bkchr mentioned this issue Apr 17, 2021
Closed
@hirschenberger
Copy link
Contributor

I'm currently working on this and different database roles are stored in the corresponding subdirectories as discussed above.

But how should we deal with existing databases to avoid users to resync their state. I suggest migrating to the new structure by:

  • checking for the existence of the said dir and if they don't exist
  • check if the current role matches the database role
  • create the new dir
  • move the existing data to the new dir.

Is it possible to move a ParityDb and RocksDb by just moving the files?

@bkchr
Copy link
Member

bkchr commented Apr 19, 2021

Is it possible to move a ParityDb and RocksDb by just moving the files?

Yes

hirschenberger added a commit to hirschenberger/substrate that referenced this issue Apr 22, 2021
@hirschenberger
Store the database in a role specific subdirectory
f4c53af 
If there is already a database in the basedir, it will be migrated
(moved) into the role-subdirectory.

fixes paritytech#6880
@hirschenberger hirschenberger mentioned this issue Apr 22, 2021
Closed
1 task
hirschenberger added a commit to hirschenberger/polkadot that referenced this issue Apr 22, 2021
@hirschenberger
Store the database in a role specific subdirectory
4e453f3 
companion PR for paritytech/substrate#6880
@hirschenberger hirschenberger mentioned this issue Apr 22, 2021
Merged
@hirschenberger
Copy link
Contributor

Can I have a review please?

@stale
Copy link

stale bot commented Jul 7, 2021

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jul 7, 2021
@hirschenberger
Copy link
Contributor

I just need a review.

@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jul 7, 2021
@hirschenberger hirschenberger mentioned this issue Jul 24, 2021
Closed
hirschenberger added a commit to hirschenberger/substrate that referenced this issue Aug 28, 2021
@hirschenberger
Store the database in a role specific subdirectory
31be7a7 
This is a cleaned up version of paritytech#8658 fixing paritytech#6880

polkadot companion: paritytech/polkadot#2923
@hirschenberger hirschenberger mentioned this issue Aug 28, 2021
Merged
@hirschenberger
Copy link
Contributor

#9645 is waiting for a review.

@ordian ordian linked a pull request Sep 7, 2021 that will close this issue
Store the database in a role specific subdirectory #9645
Merged
@ghost ghost closed this as completed in #9645 Sep 7, 2021
ghost pushed a commit that referenced this issue Sep 7, 2021
@hirschenberger
Store the database in a role specific subdirectory (#9645)
6e15de9 
* Store the database in a role specific subdirectory

This is a cleaned up version of #8658 fixing #6880

polkadot companion: paritytech/polkadot#2923

* Disable prometheus in tests

* Also change p2p port

* Fix migration logic

* Use different identification file for rocks and parity db

Add tests for paritydb migration
ghost pushed a commit to paritytech/polkadot that referenced this issue Sep 7, 2021
@hirschenberger
Store the database in a role specific subdirectory (#2923)
fb6c9b3 
* Store the database in a role specific subdirectory

companion PR for paritytech/substrate#6880

* fix test

* update Substrate

Co-authored-by: parity-processbot <>
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
U2-some_time_soon Issue is worth doing soon. Z0-trivial Writing the issue is of the same difficulty as patching the code.
Projects
None yet
Milestone
No milestone
6 participants
@rakanalh @hirschenberger @bkchr @expenses @niklasad1 @seunlanlege