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

chore: content_script_version_2: add simple protection and rm messages_backup if exists #2531

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

Ivansete-status
Copy link
Collaborator

@Ivansete-status Ivansete-status commented Mar 13, 2024

Description

This is aimed to add a simple protection. If the messages_backup exists, then delete it.
Notice that this would only protect the users that are now running a version lower than v0.26.0, and update their nodes to a version >= v0.27.0.

EDITED: In further migrations, we might need to implement a rollback feature that should trigger if an error occurs within the migration, as suggested by @SionoiS some time ago.

How to test

  1. Run a node with store protocol enabled for a while and version ' v0.25.0`. Wait until some messages are written (I've tested it with ~3.6GB database.)
  2. Run a node from this branch and

Issue

closes waku-org/nwaku-compose#75

@Ivansete-status Ivansete-status self-assigned this Mar 13, 2024
@Ivansete-status Ivansete-status marked this pull request as ready for review March 13, 2024 11:03
Copy link
Contributor

@SionoiS SionoiS left a comment

Choose a reason for hiding this comment

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

Am I understanding correctly that this would fix the case where the migration errors before DROP TABLE IF EXISTS messages_backup; at the end?

If yes then it would fix the problem but if it fails again then the backup is lost no?

Up to you if loosing the backup is an acceptable risk.

Copy link
Contributor

@NagyZoltanPeter NagyZoltanPeter left a comment

Choose a reason for hiding this comment

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

LGTM!

@Ivansete-status
Copy link
Collaborator Author

Am I understanding correctly that this would fix the case where the migration errors before DROP TABLE IF EXISTS messages_backup; at the end?

If yes then it would fix the problem but if it fails again then the backup is lost no?

Up to you if loosing the backup is an acceptable risk.

Cool thanks! I adapted it according to your advise 🥳
39b6d24

@Ivansete-status Ivansete-status merged commit c6c376b into master Mar 13, 2024
8 checks passed
@Ivansete-status Ivansete-status deleted the protect-migration branch March 13, 2024 16:18
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.

Trying to rename messages table as messages_backup when messages_backup already exists.
3 participants