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

Log env path in BerkeleyEnvironment::Flush #14760

Merged
merged 1 commit into from Dec 4, 2018

Conversation

Projects
None yet
5 participants
@promag
Copy link
Member

commented Nov 19, 2018

With bitcoind -regtest -wallet=w1 -wallet=w2 -debug, before:

BerkeleyEnvironment::Flush: Flush(true)
BerkeleyEnvironment::Flush: Flushing wallet.dat (refcount = 0)...
BerkeleyEnvironment::Flush: wallet.dat checkpoint
BerkeleyEnvironment::Flush: wallet.dat detach
BerkeleyEnvironment::Flush: wallet.dat closed
BerkeleyEnvironment::Flush: Flush(true) took              23ms
BerkeleyEnvironment::Flush: Flush(true)
BerkeleyEnvironment::Flush: Flushing wallet.dat (refcount = 0)...
BerkeleyEnvironment::Flush: wallet.dat checkpoint
BerkeleyEnvironment::Flush: wallet.dat detach
BerkeleyEnvironment::Flush: wallet.dat closed
BerkeleyEnvironment::Flush: Flush(true) took              19ms

After:

BerkeleyEnvironment::Flush: [/Users/joao/Library/Application Support/Bitcoin/regtest/wallets/w1] Flush(true)
BerkeleyEnvironment::Flush: Flushing wallet.dat (refcount = 0)...
BerkeleyEnvironment::Flush: wallet.dat checkpoint
BerkeleyEnvironment::Flush: wallet.dat detach
BerkeleyEnvironment::Flush: wallet.dat closed
BerkeleyEnvironment::Flush: Flush(true) took              23ms
BerkeleyEnvironment::Flush: [/Users/joao/Library/Application Support/Bitcoin/regtest/wallets/w2] Flush(true)
BerkeleyEnvironment::Flush: Flushing wallet.dat (refcount = 0)...
BerkeleyEnvironment::Flush: wallet.dat checkpoint
BerkeleyEnvironment::Flush: wallet.dat detach
BerkeleyEnvironment::Flush: wallet.dat closed
BerkeleyEnvironment::Flush: Flush(true) took              19ms
@promag

This comment has been minimized.

Copy link
Member Author

commented Nov 19, 2018

Not sure the best format, or if it should be in every line, but reading the current output is very confusing, wallet.dat everywhere..

@meshcollider meshcollider added the Wallet label Nov 19, 2018

@meshcollider

This comment has been minimized.

Copy link
Member

commented Nov 19, 2018

utACK 4674610

@@ -692,7 +692,7 @@ void BerkeleyEnvironment::Flush(bool fShutdown)
{
int64_t nStart = GetTimeMillis();
// Flush log data to the actual data file on all files that are not in use
LogPrint(BCLog::DB, "BerkeleyEnvironment::Flush: Flush(%s)%s\n", fShutdown ? "true" : "false", fDbEnvInit ? "" : " database not started");
LogPrint(BCLog::DB, "BerkeleyEnvironment::Flush: [%s] Flush(%s)%s\n", strPath, fShutdown ? "true" : "false", fDbEnvInit ? "" : " database not started");

This comment has been minimized.

Copy link
@practicalswift

practicalswift Nov 20, 2018

Member

Perhaps change the order so that that the general part of the message comes before the environment specific part of the message in order to increase readability?

That is change from …

BerkeleyEnvironment::Flush: [/Users/joao/Library/Application Support/Bitcoin/regtest/wallets/w1] Flush(true)

… to say …

BerkeleyEnvironment::Flush: Flush(true), file: /Users/joao/Library/Application Support/Bitcoin/regtest/wallets/w1

This comment has been minimized.

Copy link
@laanwj

laanwj Dec 4, 2018

Member

nah, context before message, message before context, you could make a valid point to go either way

@practicalswift

This comment has been minimized.

Copy link
Member

commented Nov 20, 2018

utACK 4674610 modulo nit

@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 23, 2018

utACK—yes why not
I think it would be nice to have a consistent format for wallet-related logging, specifying what wallet every log message related to, but I also understand that this is difficult to mesh with the current design, and this is an improvement.

@jonasschnelli

This comment has been minimized.

Copy link
Member

commented Nov 25, 2018

utACK 4674610

@laanwj laanwj merged commit 4674610 into bitcoin:master Dec 4, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Dec 4, 2018

Merge #14760: Log env path in BerkeleyEnvironment::Flush
4674610 Log env path in BerkeleyEnvironment::Flush (João Barbosa)

Pull request description:

  With `bitcoind -regtest -wallet=w1 -wallet=w2 -debug`, before:

  ```
  BerkeleyEnvironment::Flush: Flush(true)
  BerkeleyEnvironment::Flush: Flushing wallet.dat (refcount = 0)...
  BerkeleyEnvironment::Flush: wallet.dat checkpoint
  BerkeleyEnvironment::Flush: wallet.dat detach
  BerkeleyEnvironment::Flush: wallet.dat closed
  BerkeleyEnvironment::Flush: Flush(true) took              23ms
  BerkeleyEnvironment::Flush: Flush(true)
  BerkeleyEnvironment::Flush: Flushing wallet.dat (refcount = 0)...
  BerkeleyEnvironment::Flush: wallet.dat checkpoint
  BerkeleyEnvironment::Flush: wallet.dat detach
  BerkeleyEnvironment::Flush: wallet.dat closed
  BerkeleyEnvironment::Flush: Flush(true) took              19ms
  ```

  After:
  ```
  BerkeleyEnvironment::Flush: [/Users/joao/Library/Application Support/Bitcoin/regtest/wallets/w1] Flush(true)
  BerkeleyEnvironment::Flush: Flushing wallet.dat (refcount = 0)...
  BerkeleyEnvironment::Flush: wallet.dat checkpoint
  BerkeleyEnvironment::Flush: wallet.dat detach
  BerkeleyEnvironment::Flush: wallet.dat closed
  BerkeleyEnvironment::Flush: Flush(true) took              23ms
  BerkeleyEnvironment::Flush: [/Users/joao/Library/Application Support/Bitcoin/regtest/wallets/w2] Flush(true)
  BerkeleyEnvironment::Flush: Flushing wallet.dat (refcount = 0)...
  BerkeleyEnvironment::Flush: wallet.dat checkpoint
  BerkeleyEnvironment::Flush: wallet.dat detach
  BerkeleyEnvironment::Flush: wallet.dat closed
  BerkeleyEnvironment::Flush: Flush(true) took              19ms
  ```

Tree-SHA512: f90b413cca5d2527324a264ce371dc8baba69f5b541f7d7f6238a8dd79398cbd3c67c0d7a8a0b69aec6c44d77ba26a079c2241427e3669ed22c7da0e4d60039e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.