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

Panic if there's a datastore err when writing state to datastore after chain msg #2870

Closed
wants to merge 1 commit into from

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Aug 6, 2020

Currently when we send a create channel or add funds message to chain, we write a record of that message into the datastore, so that if lotus is shutdown or crashes, we can start tracking the message again when lotus comes back up.

Ideally we would have a transactional mechanism in place that would

  • write "sending message" to datastore
  • send message
  • write "message sent" to datastore

and then be able to recover from incomplete states by inspecting the datastore and the chain.

Without having this transactional mechanism in place, the correct solution is probably to panic when there's a datastore error, because otherwise the system will be in an "undefined" state.

Resolves #2640 (review)

…to the datastore when we've already changed chain state
@dirkmc dirkmc changed the base branch from master to next August 6, 2020 17:49
@dirkmc dirkmc changed the title Panic if there's a datastore when writing state to datastore after chain msg Panic if there's a datastore err when writing state to datastore after chain msg Aug 6, 2020
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Crashing the node is definitely not the correct answer here. Missing some payment channel info may be expensive, but still much less than having the node crash in the middle of some other very expensive operation (submitting a block, submitting window PoSt barely on time, etc.)

@dirkmc
Copy link
Contributor Author

dirkmc commented Aug 6, 2020

Fair enough 👍

I will change to just log and return

@dirkmc dirkmc closed this Aug 6, 2020
@daviddias daviddias deleted the fix/panic-on-ds-err branch August 19, 2020 18:33
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.

2 participants