Skip to content

[WIP] Batched payment deletion#5368

Closed
guggero wants to merge 2 commits into
lightningnetwork:masterfrom
guggero:batched-payment-deletion
Closed

[WIP] Batched payment deletion#5368
guggero wants to merge 2 commits into
lightningnetwork:masterfrom
guggero:batched-payment-deletion

Conversation

@guggero
Copy link
Copy Markdown
Collaborator

@guggero guggero commented Jun 9, 2021

Attempts to fix #5349 by batching the delete operations of DeletePayments.

This makes the operation no longer atomic. But that might be okay... Or do we need to remove the payment bucket and indices in the same batch? Perhaps we need to do "horizontal" batching instead of "vertical"?

guggero added 2 commits June 9, 2021 13:18
As a preparation of using multiple batches to delete the payments we
begin with extracting the pure read into its own read transaction.
To reduce the size of the delete transactions, we batch the delete
operations into batches of 1k. This should speed up the deletion process
considerably as not all changes need to be kept in memory as a whole.
We also add some debug log statements about the process so a user can
see what's happening.
@Roasbeef Roasbeef modified the milestones: v0.14.0, 0.13.1 Jun 14, 2021
Copy link
Copy Markdown
Collaborator

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

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

I think that we can get into a bad state if we delete payment and index entry separately.

Taking the following example:

  • We delete payment with index 3
  • DeletePayments is killed before we remove the index entry
  • Next time call DeletePayments, the payment no longer exists so we don't queue its index for deletion

This will break calls to QueryPayments because we'll scan the index bucket, land on the index for 3 then try to lookup a payment that doesn't exist which will fail the call.

I think we could change the ordering to delete indexes first, but then we could end up with a weird case where the payment is no longer included in QueryPayments but it's actually still in the payments bucket. That would be resolved if we ran DeletePayments again, but may not be desirable to allow this kind of state to be a possibility.

Comment thread channeldb/payments.go
return nil
})
}, func(){})
}, func() {})
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.

nit: previous commit

@Roasbeef Roasbeef modified the milestones: 0.13.1, v0.14.0 Jul 1, 2021
@Roasbeef Roasbeef modified the milestones: v0.14.0, v0.15.0 Aug 30, 2021
@lightninglabs-deploy
Copy link
Copy Markdown
Collaborator

@guggero, remember to re-request review from reviewers for your latest update

@guggero
Copy link
Copy Markdown
Collaborator Author

guggero commented Dec 7, 2021

Individual payments can now be deleted, so the same effect can be achieved in a loop. Closing this until we decide we really need the functionality.

@guggero guggero closed this Dec 7, 2021
@guggero guggero deleted the batched-payment-deletion branch March 11, 2022 10:57
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.

deleteFailedPayment offline?

4 participants