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

Auto-rename of db_dir/mainnet to db_dir/bitcoin #513

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kixunil
Copy link
Contributor

@Kixunil Kixunil commented Sep 29, 2021

Since version 0.9 the mainnet subdir is called bitcoin we need to
rename it if auto reindex is specified to fulfill the purpose of auto
reindex. The logic in this change keeps the intention of disabled auto
reindex causing no changes.

@Kixunil Kixunil mentioned this pull request Sep 29, 2021
src/bin/electrs.rs Outdated Show resolved Hide resolved
@Kixunil
Copy link
Contributor Author

Kixunil commented Sep 29, 2021

Sorry about failed fmt. I would do it right away but I have to go AFK. I'm not certain I will be able to fix it up today because of unexpected health issues. :(

@romanz
Copy link
Owner

romanz commented Sep 29, 2021

Hope you get well soon!
Unfortunately, I won't be able to test the PR today - I will add a note to the release notes about this issue.

@romanz
Copy link
Owner

romanz commented Sep 29, 2021

Added 8b8ebea.

Kixunil added a commit to Kixunil/electrs that referenced this pull request Sep 29, 2021
This warns users that they shouldn't touch `db_dir` internals except for
deleting the old `mainnet` database. It explains in details how to do
upgrade and deleting properly, calls it out in release notes and fixes
small style mistake in nearby heading.

It can be thought of as more explicit/detailed version of
8b8ebea or a textual alternative to
PR romanz#513.
@Kixunil
Copy link
Contributor Author

Kixunil commented Sep 29, 2021

Health issue fixed. :)

I decided to make #514 in case this one doesn't make it into 0.9 as that one is just doc change. It will conflict with 8b8ebea but should be very easy to resolve.

@schildbach
Copy link
Contributor

I'm not sure if this is a good idea. Data volumes are typically mounted from outside of the container, so it can't simply be renamed from within.

@Kixunil
Copy link
Contributor Author

Kixunil commented Oct 5, 2021

@schildbach you should mount the top-level directory (the one you supply in configuration), not internal directories. Mounting internal directories is equal to messing with internal structures.

@schildbach
Copy link
Contributor

@Kixunil I don't need the "testnet"/"mainnet" directories in my data volumes. I'm running separated containers and separated volumes for different chains already, so I mounted the specific data directories explicitly.

@Kixunil
Copy link
Contributor Author

Kixunil commented Oct 5, 2021

@schildbach frankly I don't like forcing subdirectories either. It'd be cleaner to just have a single directory and let the user worry about different instances. But given where we're today I don't think this can be changed without major disruption (just look at the spike of issues since 0.9 was released). And even if it was, I'd still prefer having a subdirectory for RocksDb, so that we can have database-independent version file at top-level and handle migrations cleanly (I was seriously considering proposing this change recently.)

If we decide to change you'd be affected with your current setup. Please note that we never documented that the contents of directory is considered stable and we actually currently document the opposite. As much as I can feel your pain, I believe it'd be wise for you to modify your setup to treat the db_dir as opaque.

@schildbach
Copy link
Contributor

@Kixunil Thanks for your suggestion. I've just changed my configuration accordingly (still on 0.8.11).

@schildbach
Copy link
Contributor

Shouldn't this be merged soon, so that people can upgrade to 0.9?

@Kixunil
Copy link
Contributor Author

Kixunil commented Feb 21, 2022

Rebased and refactored to use a separate function.

Since version 0.9 the `mainnet` subdir is called `bitcoin` we need to
rename it if auto reindex is specified to fulfill the purpose of auto
reindex. The logic in this change keeps the intention of disabled auto
reindex causing no changes.
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.

3 participants