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.

Sign up for GitHub

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

Jump to bottom

Replay events on event handling failures due to persistence failures. #374

Merged
merged 11 commits into from
Oct 16, 2024
Merged

Replay events on event handling failures due to persistence failures. #374

merged 11 commits into from
Oct 16, 2024

Conversation

G8XSU
Copy link
Contributor

@G8XSU G8XSU commented Oct 14, 2024 •
edited
Loading

Handle 3 types of failures mainly:

  • Failed to update payment_store due to persistence failure.
  • Failed to push ldk_node event to event_queue due to persistence failure.
  • Output tracking failure in output_sweeper due to persistence failure.

@G8XSU G8XSU requested a review from tnull October 14, 2024 19:41
@G8XSU G8XSU mentioned this pull request Oct 14, 2024
Merged
@G8XSU G8XSU changed the title Replay events when there are failures due to persistence failure. Replay events on event handling failures due to persistence issues. Oct 14, 2024
@G8XSU G8XSU changed the title Replay events on event handling failures due to persistence issues. Replay events on event handling failures due to persistence failures. Oct 14, 2024
tnull
tnull reviewed Oct 15, 2024
View reviewed changes
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Generally looks good as a first step, two comments though.

In a follow-up (probably based on/after #365 landed) we should try to tackle as many of the remaining explicit panics on persistence failure as possible.

src/event.rs Outdated Show resolved Hide resolved
src/event.rs Outdated Show resolved Hide resolved
@tnull
Copy link
Collaborator

tnull commented Oct 15, 2024

Also needs a rebase to account for the yanked 0.0.124.

@G8XSU
Copy link
Contributor Author

G8XSU commented Oct 15, 2024

Rebased and addressed comments.

@G8XSU G8XSU requested a review from tnull October 15, 2024 22:56
tnull
tnull approved these changes Oct 16, 2024
View reviewed changes
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM

@tnull tnull merged commit cffdf7e into lightningdevkit:main Oct 16, 2024
12 of 13 checks passed
This was referenced Oct 16, 2024
Closed
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnull tnull tnull approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@G8XSU @tnull