Decayed log optional migration

[ziggie1984]

Builds on top of https://github.com/lightningnetwork/lnd/pull/9929.

This PR adds an Optional Migration which is by default set to true.

~~Looking for ACK or NACK.~~

[coderabbitai[bot]]

[!IMPORTANT]

Review skipped

Auto reviews are limited to specific labels.

:label: Labels to auto review (1)

• llm-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[ziggie1984]

After migration:

[saubyk]

This PR adds an Optional Migration which is by default set to true.

If it's an optional migration, shouldn't it be false by default?

[ziggie1984]

If it's an optional migration, shouldn't it be false by default?

Good point, I decided to make it true by default because there is absolutely no reason to keep it (the data which is deleted in the migration). I choose the optional migration over the mandatory one overall because it is the less intrusive way into the code base, and because it is a minor release people can easy downgrade back to 19.1/19.0 if something is wrong with the migration. So I think it is totally fine having this default set to true.

[ziggie1984]

Ok this PR still has one Question to resolve:

This PR introduces a protection mechanism so that people upgrade to a newer version with a new Optional Migration applied cannot just downgrade their software because then we have to way to way to apply the optional migration again because we map it in the Optional Metadata. But we still need to solve the problem when people startup 19.2 and then for whatever reason downgrade to 19.1 again. This is not covered yet, probably we need to add a mandatory migration to prevent this ?

Or we make every Optional Migration idempotent and every migration makes sure it is idempotent when called more than once and therefore we can remove the Optional Metadata bucket.

[ziggie1984]

This PR introduces a protection mechanism so that people upgrade to a newer version with a new Optional Migration applied cannot just downgrade their software because then we have to way to way to apply the optional migration again because we map it in the Optional Metadata. But we still need to solve the problem when people startup 19.2 and then for whatever reason downgrade to 19.1 again. This is not covered yet, probably we need to add a mandatory migration to prevent this ?

Solved this by counting up the migration number for migration 34. So this change will disallow LND users to revert back to previous versions.

[ziggie1984]

Talked also to @guggero and the way forward here. We decided to make this migration truly optional, because nothing will get broke if the user for whatever reason downgrades from 19.2 back to 19.1 or earlier. And because everything will be SQLized in the near term we will eventually clean "garbage data". This approach is also in line with our general policy of not adding mandatory DB migrations for minor releases.

[saubyk]

We decided to make this migration truly optional

so, optional as in opt-in or opt-out? discussed with @Roasbeef yesterday and we think this should be an opt-in migration.

[guggero]

We decided to make this migration truly optional

so, optional as in opt-in or opt-out? discuss with @Roasbeef yesterday and we think this should be an opt-in migration.

Currently it's opt-out, but we can change that if we feel most users won't want to run it.

[ziggie1984]

I would say we are ok with selecting the opt-out method because with the new approach the migration is not mandatory in a sense that you cannot revert to a previous version. Therefore if something happens with 19.2 the user can just go back to 19.1. As mentioned above the only small disadventage this approach has is, that if the user then goes back to 19.2 or later it will not do the optional migration anymore leaving an unused bucket in the storage. Which isn't a problem because it is not much data and will be garbage-collected when we go to native SQL. I would like to run the garbage collection by default because it removes a lot of disk-usage for big nodes and speeds things up for SQL postgres because no index is required anymore.

[Roasbeef]

Can be rebased now that the dep PR was merged!

[Roasbeef]

I think compared to other migrations the actually read/write a large amount of keys, this migration should be much faster (at least on bbolt), as bucket deletion just mark a page on disk as free, and doesn't actually delete until one runs a compaction.