Skip to content

Allow to cancel the txdb upgrade via splashscreen keypress 'q'#10660

Merged
laanwj merged 6 commits intobitcoin:masterfrom
jonasschnelli:2017/06/chainstate_update_prog
Jun 29, 2017
Merged

Allow to cancel the txdb upgrade via splashscreen keypress 'q'#10660
laanwj merged 6 commits intobitcoin:masterfrom
jonasschnelli:2017/06/chainstate_update_prog

Conversation

@jonasschnelli
Copy link
Contributor

Progress calc credit: @sipa

This PR will accomplish three things:

  • Show the progress of the UTXO-db upgrade in the Qt splashscreen and in the debug log:
    • 2017-06-23 07:49:55 Upgrading utxo-set database...
    • 2017-06-23 07:49:55 [0%]...[0%]...[13%]...[25%]...[39%]...[52%]...[63%]...[76%]...[90%]...[DONE].
  • Allow to shutdown during the UTXO-db upgrade (ctrl-c, Qt quit)
  • Allow to press q in splash screen to shutdown.

Let me explain why keypress instead of buttons:
The current splash screen is a drawn pixmap. Adding buttons there will require a complete re-design of the splashscreen (should be done once). It should be an auto-resizing QWidget as used in the other layout situations.

For getting a UTXO-upgrade abort into 0.15, this solution may be acceptable.

@jonasschnelli jonasschnelli force-pushed the 2017/06/chainstate_update_prog branch 3 times, most recently from 90ebd98 to 8aaf606 Compare June 23, 2017 07:53
@jonasschnelli
Copy link
Contributor Author

Speak up if you have a better, state-less solution for the UI progress callback.

@laanwj
Copy link
Member

laanwj commented Jun 23, 2017

Awesome! Will test.

The current splash screen is a drawn pixmap. Adding buttons there will require a complete re-design of the splashscreen (should be done once). It should be an auto-resizing QWidget as used in the other layout situations.

TBH this doesn't matter much. This is a one-time scenario, it's important to have something that works reliably, it doesn't need to look pretty.

@jonasschnelli jonasschnelli changed the title Allow to cancel the txdb upgrade via splashscreen keypress 's' Allow to cancel the txdb upgrade via splashscreen keypress 'q' Jun 23, 2017
src/txdb.cpp Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this just be one string with a \n in the middle instead of two separate strings?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my experience, translators hate messages with html or newlines in them, increasing the chance to make a mistake there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shut down as two words maybe?

@sipa
Copy link
Member

sipa commented Jun 23, 2017

Tried this. Two issues:

  • 2017-06-23 21:15:54 [15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%]...[15%].... We should probably only print at most once for each %.
  • There seems to be a space missing between the message and the percentage in the spash screen.

@jonasschnelli jonasschnelli force-pushed the 2017/06/chainstate_update_prog branch 2 times, most recently from 5268f8d to 5b304a1 Compare June 28, 2017 19:30
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Jun 28, 2017

Rebased and overhauled.
Addressed @sipa's points.
Also reports now [CANCELLED] instead of the [DONE] when shutting down during txdb upgrade.

@laanwj
Copy link
Member

laanwj commented Jun 29, 2017

Tested upgrade w/ my test data up to height=430234:

  • quitting with q worked, it indeed continues on next start
  • killing with kill -9 also made it continue at next start without problems
  • progress logging in debug.log is as expected
  • upgrade succeeded, node is running

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer breakAction to 'continueAction'.
Continue, to me, implies that it's part of a pause/continue kind of functionality, not early quit.

@jonasschnelli jonasschnelli added this to the 0.15.0 milestone Jun 29, 2017
@jonasschnelli jonasschnelli force-pushed the 2017/06/chainstate_update_prog branch from 5b304a1 to c7e9233 Compare June 29, 2017 15:45
@jonasschnelli
Copy link
Contributor Author

Agree with @laanwj, changed continueAction (and similar function names) to breakAction.
(amend commits / force push)

@jonasschnelli jonasschnelli force-pushed the 2017/06/chainstate_update_prog branch from c7e9233 to b96ce7d Compare June 29, 2017 15:47
@jonasschnelli jonasschnelli force-pushed the 2017/06/chainstate_update_prog branch from b96ce7d to 542ce6e Compare June 29, 2017 15:48
@laanwj laanwj self-assigned this Jun 29, 2017
@laanwj
Copy link
Member

laanwj commented Jun 29, 2017

ACK 542ce6e

@laanwj laanwj merged commit 542ce6e into bitcoin:master Jun 29, 2017
laanwj added a commit that referenced this pull request Jun 29, 2017
…ess 'q'

542ce6e Report [CANCELLED] instead of [DONE] when shut down during txdb upgrade (Jonas Schnelli)
83fbea3 Report txdb upgrade not more often then every 10% (Jonas Schnelli)
06c5b6e Show txdb upgrade progress in debug log (Jonas Schnelli)
316fcb5 Allow to cancel the txdb upgrade via splashscreen callback (Jonas Schnelli)
ae09d45 Allow to shut down during txdb upgrade (Jonas Schnelli)
00cb69b [Qt] allow to execute a callback during splashscreen progress (Jonas Schnelli)

Tree-SHA512: 23190f23f441bfd60821e49f8b3698a6bef97eb0e0ee659328e4a7395769ecd1616420eacc38aa1fa0ff62b9de5f13a0098dc798cdec6bff649575cefebc0db2
@TheBlueMatt
Copy link
Contributor

utACK with the changes at #10770.

codablock pushed a commit to codablock/dash that referenced this pull request Oct 26, 2017
…n keypress 'q'

542ce6e Report [CANCELLED] instead of [DONE] when shut down during txdb upgrade (Jonas Schnelli)
83fbea3 Report txdb upgrade not more often then every 10% (Jonas Schnelli)
06c5b6e Show txdb upgrade progress in debug log (Jonas Schnelli)
316fcb5 Allow to cancel the txdb upgrade via splashscreen callback (Jonas Schnelli)
ae09d45 Allow to shut down during txdb upgrade (Jonas Schnelli)
00cb69b [Qt] allow to execute a callback during splashscreen progress (Jonas Schnelli)

Tree-SHA512: 23190f23f441bfd60821e49f8b3698a6bef97eb0e0ee659328e4a7395769ecd1616420eacc38aa1fa0ff62b9de5f13a0098dc798cdec6bff649575cefebc0db2
codablock pushed a commit to codablock/dash that referenced this pull request Oct 31, 2017
…n keypress 'q'

542ce6e Report [CANCELLED] instead of [DONE] when shut down during txdb upgrade (Jonas Schnelli)
83fbea3 Report txdb upgrade not more often then every 10% (Jonas Schnelli)
06c5b6e Show txdb upgrade progress in debug log (Jonas Schnelli)
316fcb5 Allow to cancel the txdb upgrade via splashscreen callback (Jonas Schnelli)
ae09d45 Allow to shut down during txdb upgrade (Jonas Schnelli)
00cb69b [Qt] allow to execute a callback during splashscreen progress (Jonas Schnelli)

Tree-SHA512: 23190f23f441bfd60821e49f8b3698a6bef97eb0e0ee659328e4a7395769ecd1616420eacc38aa1fa0ff62b9de5f13a0098dc798cdec6bff649575cefebc0db2
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants