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

[Epic] channeldb+payment: create native SQL schema for payment storage #7920

Open
2 tasks
yyforyongyu opened this issue Aug 24, 2023 · 1 comment
Open
2 tasks
Assignees
Labels
database Related to the database/storage of LND epic Issues created to track large feature development P1 MUST be fixed or reviewed payments Related to invoices/payments sql

Comments

@yyforyongyu
Copy link
Member

yyforyongyu commented Aug 24, 2023

Depends on #6288.

This is the master issue that tracks the progress of adding native SQL support for payment db, which is split into three steps.

Support native SQL config

The first step is to allow users to configure SQL backends. There are two approaches,

  1. use existing config flags.
  2. add new config flags.

Since we already support multiple database backends, current nodes running with SQL would already have tables filled with key-value data. Thus going with option 1 creates more complexity as now we need to deal with mixed data types in the same database. Option 2 allows managing the native SQL in a clean state. With a few flag tricks, we can easily separate the way the database is configured. The idea is expressed in

Use native SQL for payments

The second step is to build SQL queries for payment data using sqlc. We already have the interface ControlTower that covers db operations. Ideally, we just need to create a new package payments to hold the data structs and db operations, to avoid we end up having a big package like channeldb. The tracking PR is,

Going SQL means we are giving up the flexibility provided by TLV, any updates to the scheme now need a migration. Future payment format should define its own struct and save the data in a new table.

Migrate kvdb data into SQL

In #6469 we introduced optional migrations. For existing nodes, we'd need to migrate their current key-value data into SQL rows. This migration needs to be paid attention to OOM as payment data can be large.

  • Finalize the optional migration scheme. The current optional migration scheme is half-finish as it only works for a single migration. To enable multiple migrations, we'd need to decide the relationship among the potential optional migrations - whether it's ok to only perform one or a few of them? Can any of the migrations be skipped? To reduce complexity, we could make it all or nothing - either we optionally run them all, or we don't run at all.
  • Add optional migration for payments. We'll need to start two database backends to move the data from kvdb to SQL. We need futher investigate this as our current migration scheme only supports one database.
@yyforyongyu yyforyongyu added database Related to the database/storage of LND payments Related to invoices/payments sql labels Aug 24, 2023
@Roasbeef
Copy link
Member

Going SQL means we are giving up the flexibility provided by TLV, any updates to the scheme now need a migration. Future payment format should define its own struct and save the data in a new table.

If relevant, we do have the ability to sort of mix things a bit, by having a column that actually stores TLV data. The upside of just using the normal migrations there though as we have to maintain less specific migration code for the various data structures.

@saubyk saubyk added this to lnd v0.18 Aug 26, 2023
@saubyk saubyk moved this to 🏗 In progress in lnd v0.18 Aug 26, 2023
@saubyk saubyk moved this from 🏗 In progress to 📋 Backlog in lnd v0.18 Nov 1, 2023
@saubyk saubyk removed this from lnd v0.18 Nov 5, 2023
@saubyk saubyk added the P1 MUST be fixed or reviewed label Nov 5, 2023
@saubyk saubyk added the epic Issues created to track large feature development label Aug 20, 2024
@ziggie1984 ziggie1984 assigned ziggie1984 and unassigned yyforyongyu Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Related to the database/storage of LND epic Issues created to track large feature development P1 MUST be fixed or reviewed payments Related to invoices/payments sql
Projects
None yet
Development

No branches or pull requests

4 participants