Make lsn_reset ("detach databases") optional and off by default.

sipa commented at 9:39 pm on April 17, 2012: member

Add an option -detachdb (and entry in OptionDialog), without which no lsn_reset is called on addr.dat and blkindex.dat. That means these files cannot be moved to a new environment, but shutdown can be significantly faster. The wallet file is always lsn_reset’ed.

-detachdb corresponds to the old behaviour, though it is off by default now to speed up shutdowns.

sipa commented at 10:50 pm on April 17, 2012: member

Extra information: lsn_reset is not faster when it is run a second time. I assume it needs to traverse the entire database file.

laanwj commented at 5:35 am on April 19, 2012: member

Code is ACK

Maybe we could add some text to the description/tooltip that it doesn’t affect the safety of the wallet. Somehow this “rush database shutdown” manages to creep me out (and that would even more apply to users, I think).

sipa commented at 11:12 am on April 19, 2012: member

@laanwj I’m not so sure - I believe many corruptions are caused by people shutting down their computers before lsn_reset has completed, or otherwise killing it because of slow shutdown. Not calling lsn_reset is perfectly safe, only you can’t move blkindex.dat to a new environment afterwards. If you do so anyway, if will fail (deterministically) at startup.

EDIT: right, your comment was about users reacting the same way. Let’s see if I can change the tooltip.

laanwj commented at 11:43 am on April 19, 2012: member

Another random concern: could this interfere with deleting db logs on startup/shutdown? (i’m not sure we do this but remember reading about it on irc) Is the “unflushed state” in the log files?

sipa commented at 12:29 pm on April 19, 2012: member

If you manually delete log files, and no lsn_reset was performed on blkindex.dat, it will fail to load. We call some bdb functions to remove log files automatically, but this only happens when they are safe to be removed.

gavinandresen commented at 0:01 am on April 22, 2012: contributor

A random thought after reading http://dbaspot.com/berkeley-db/266028-database-recovery-no-transaction-logs-sybase-its-termed-suicide-recovery.html

If the datadir/database directory is empty or missing at startup, could we just leave the DB_RECOVER flag out of the dbenv.open() call and cross our fingers that the .dat files are in a consistent state? Or maybe do whatever db_verify does and exit cleanly if they are NOT in a consistent state?

sipa commented at 0:37 am on April 22, 2012: member

I did a test: close all database files nicely, do a checkpoint, (do not reset lsn’s), shutdown the database environment cleanly, delete the log files, restart application without DB_RECOVER. Result: Db::open: Invalid argument.

So it seems that no matter what, the log file needs to be present. I wonder how db_dump works, because that tool can dump the records without any problems in this situation.

gavinandresen commented at 7:56 pm on April 23, 2012: contributor

ACK.

db_dump calls: dbp->verify(dbp, filename, NULL, stdout, DB_SALVAGE | (Rflag ? DB_AGGRESSIVE : 0) | (pflag ? DB_PRINTABLE : 0));

sipa commented at 8:08 pm on April 23, 2012: member

Right, not what we want to do by ourself. @jgarzik and @gmaxwell: what worries do you still have w.r.t. this pullreq?

sipa commented at 7:04 pm on April 24, 2012: member

@laanwj If i’d change the checkbox to be “detach block and address databases from environment (slower shutdown)”, and make it default off. Would that sound less alarming?

sipa commented at 8:38 pm on April 25, 2012: member

I swapped -fastshutdown=1 for -detachdb=0. This way of formulating things hopefully sounds less scary.

Diapolo commented at 8:57 pm on April 25, 2012: none

The blk000x.dat file(s) are not involved here, as you didn’t mention it? As addr.dat and blkindex.dat can be rebuild I think it is a nice addition / option. I had to read the description several times to understand what is default now ^^.

Make lsn_reset ("detach databases") optional and off by default.

Add an option -detachdb (and entry in OptionDialog), without which no
lsn_reset is called on addr.dat and blkindex.dat. That means these
files cannot be moved to a new environment, but shutdown can be
significantly faster. The wallet file is always lsn_reset'ed.

-detachdb corresponds to the old behaviour, though it is off by
default now to speed up shutdowns.

83743ed681

in src/init.cpp: in 541fd70788 outdated

199@@ -200,6 +200,7 @@ bool AppInit2(int argc, char* argv[])
200 #else
201             "  -upnp            \t  "   + _("Use Universal Plug and Play to map the listening port (default: 0)") + "\n" +
202 #endif
203+            "  -detachdb        \t  "   + _("Detach block and address databases. Increases shutdown time (default: 9)") + "\n" +

Diapolo commented at 9:00 pm on April 25, 2012:

If detachdb is a bool value, how can the default be 9?

sipa commented at 10:32 pm on April 25, 2012:

Thanks for noticing, it’s a typo :)

sipa commented at 10:34 pm on April 25, 2012: member

Default is the fast shutdown without calling lsn_reset on blkindex.dat and addr.dat. I believe that’s what most people want - very few people move these files around between environments.

blk0001.dat is indeed not involved here as it is no database.

Diapolo commented at 4:59 am on April 26, 2012: none

Thanks for clarifying and ACK ;).

laanwj commented at 5:47 am on April 26, 2012: member

I like the diplomatic renaming :-) ACK.

sipa referenced this in commit e1ea3ce7aa on Apr 26, 2012

sipa merged this on Apr 26, 2012

sipa closed this on Apr 26, 2012

coblee referenced this in commit 167208088f on Jul 17, 2012

sipa deleted the branch on Jun 23, 2017

suprnurd referenced this in commit 4894345a58 on Dec 5, 2017

lateminer referenced this in commit 93fb1bcaa0 on Jan 22, 2019

lateminer referenced this in commit 6ceff8f97f on Dec 25, 2019

Bushstar referenced this in commit 5c7bc5f6f3 on May 5, 2020

DrahtBot locked this on Sep 8, 2021

Make lsn_reset (“detach databases”) optional and off by default. #1119