Skip to content

feat(dashmate): add core reindex command - #533

Merged
shumkov merged 44 commits into
masterfrom
feat/dashmate-reindex
Oct 3, 2022
Merged

feat(dashmate): add core reindex command#533
shumkov merged 44 commits into
masterfrom
feat/dashmate-reindex

Conversation

@pshenmic

@pshenmic pshenmic commented Sep 22, 2022

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

Sometimes, dashcore nodes require reindex (rescan of blockchain) in order to recover or find missing transactions.

When you are using standalone dashcore, you have to manually edit dashcore configuration files and run nodes in reindex mode. Then get it back after the operation succeeded.

Using dashmate, we run everything in docker and it is hard to achieve that without dropping the whole chain and messing with containers. To accomplish that, you need to edit templates, and do some tricky stuff.

What was done?

Added new command reindex for both regular and local configurations, which runs core nodes from your configuration in the reindex mode. It modifies the configuration of dashcore, start the core, wait until it finishes sync, and turn configuration back again.

Implementation Details

New configuration keys are introduced in the config: core.reindex, core.reindexContainerId. When reindex command is called, it sets core.reindex to 1, which renders new dash.conf file in your dashmate folder, and start the core via startCore function. Container id of started container saved in the configuration under core.reindexContainerId path. It is used for the command interruption. You can ctrl + c any time, it will pick up container next run. Then the command will connect to the core and wait until the blockchain is fully synced. Once dashcore reach the head of blockchain, the command will stop it back, and reset core mode back to 0.

You cannot start configartion until resync is fully complete

How Has This Been Tested?

Locally

Breaking Changes

  • Added new required core.reindex and optionsl core.reindexContainerId configuration key

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@pshenmic
pshenmic changed the base branch from v0.23-dev to master September 22, 2022 09:29
@pshenmic
pshenmic marked this pull request as draft September 22, 2022 09:30
@lgtm-com

lgtm-com Bot commented Sep 22, 2022

Copy link
Copy Markdown

This pull request introduces 5 alerts when merging 397a456 into 0102380 - view on LGTM.com

new alerts:

  • 3 for Unused variable, import, function or class
  • 2 for Missing variable declaration

Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
Comment thread packages/dashmate/src/listr/tasks/startNodeTaskFactory.js Outdated
@pshenmic
pshenmic marked this pull request as ready for review September 23, 2022 07:32
@pshenmic
pshenmic requested a review from strophy September 23, 2022 07:36
@lgtm-com

lgtm-com Bot commented Sep 23, 2022

Copy link
Copy Markdown

This pull request introduces 5 alerts when merging 0ca2433 into 0102380 - view on LGTM.com

new alerts:

  • 3 for Unused variable, import, function or class
  • 2 for Missing variable declaration

@shumkov shumkov changed the title feat(dashmate): add 'reindex' command feat(dashmate): add 'reindex command Sep 30, 2022
@shumkov shumkov changed the title feat(dashmate): add 'reindex command feat(dashmate): add core reindex command Sep 30, 2022
@lgtm-com

lgtm-com Bot commented Sep 30, 2022

Copy link
Copy Markdown

This pull request introduces 2 alerts when merging 1a27d86 into dbf3560 - view on LGTM.com

new alerts:

  • 2 for Missing variable declaration

@shumkov shumkov added this to the v0.23.0 milestone Sep 30, 2022

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
Comment thread packages/dashmate/configs/migrations.js Outdated
Comment thread packages/dashmate/src/commands/core/reindex.js Outdated
Comment thread packages/dashmate/src/commands/core/reindex.js Outdated
Comment thread packages/dashmate/src/commands/group/core/reindex.js Outdated
Comment thread packages/dashmate/src/commands/group/core/reindex.js Outdated
@shumkov
shumkov self-requested a review September 30, 2022 13:55

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👆

@lgtm-com

lgtm-com Bot commented Sep 30, 2022

Copy link
Copy Markdown

This pull request introduces 2 alerts when merging 47e46f7 into dbf3560 - view on LGTM.com

new alerts:

  • 2 for Missing variable declaration

pshenmic and others added 2 commits October 1, 2022 02:16
@pshenmic
pshenmic requested a review from shumkov September 30, 2022 19:17
@lgtm-com

lgtm-com Bot commented Sep 30, 2022

Copy link
Copy Markdown

This pull request introduces 3 alerts when merging 60edd37 into dbf3560 - view on LGTM.com

new alerts:

  • 2 for Missing variable declaration
  • 1 for Overwritten property

Comment thread packages/dashmate/README.md Outdated
Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
@lgtm-com

lgtm-com Bot commented Sep 30, 2022

Copy link
Copy Markdown

This pull request introduces 3 alerts when merging a7edbfc into dbf3560 - view on LGTM.com

new alerts:

  • 2 for Missing variable declaration
  • 1 for Overwritten property

@lgtm-com

lgtm-com Bot commented Sep 30, 2022

Copy link
Copy Markdown

This pull request introduces 3 alerts when merging c4d855a into dbf3560 - view on LGTM.com

new alerts:

  • 2 for Missing variable declaration
  • 1 for Overwritten property

@pshenmic
pshenmic requested a review from shumkov October 3, 2022 05:59
Comment thread packages/dashmate/configs/migrations.js Outdated
Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js Outdated
Comment thread packages/dashmate/src/listr/tasks/reindexNodeTaskFactory.js
@pshenmic
pshenmic requested a review from shumkov October 3, 2022 14:43

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good job! 👍

@shumkov
shumkov merged commit 1960a1e into master Oct 3, 2022
@shumkov
shumkov deleted the feat/dashmate-reindex branch October 3, 2022 17:39
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.

4 participants