badger icon indicating copy to clipboard operation
badger copied to clipboard

fix(PanicHandling): Add Options.PanicHandler

Open VinGarcia opened this issue 2 years ago • 9 comments

Problem

This PR fixes #1883.

The issue is that when a device running badger is abruptly powered off it might cause a database corruption, and subsequently badger will continuously panic from inside a separate Goroutine when trying to open the database.

Panicking from inside a Goroutine causes all other Goroutines to die with an error code, and the main isssue is that having a defer calling recover() on other Goroutines cannot prevent this from happening, making it extremely difficult to recover from such a failure if the affected developer has no control over the panicking goroutine.

Solution

The solution proposed here is to create all Goroutines with a special method called DB._go() that will recover from the panic if the user provides a PanicHandler on the Options argument.

If a user decides to use this feature he should make sure not to ignore the panic, but this would give a window for developers to work around this issue by for example deleting the offending database and creating a new one, or even just reporting an error to the appropriate API so a human can be notified of this issue.

The main complexity in this PR is the issue regarding the use of closure and Goroutines inside for loops:

https://go.dev/doc/faq#closures_and_goroutines

Before this PR these issues were handled by passing the arguments to the Goroutines as in:

for i := 0; i < n; i++ {
    go someFunc(i)
    // or
    go func(threadID int) {
        doSomethingWith(threadID)
    }(i)
}

Due to the way the DB._go() function was implemented (i.e. receiving a closure as argument) it is not possible anymore to trust this technique to make sure the data is copied and not passed as a reference to the closure, so instead I am doing it like this now wherever necessary:

for i := 0; i < n; i++ {
    i := i // This effectively creates a copy of i that will not be overwritten after this iteration
    db._go(func() {
        someFunc(i)
    }
    // or
    threadID := i
    db._go(func() {
        doSomethingWith(threadID)
    })
}

VinGarcia avatar Dec 07 '23 14:12 VinGarcia

Deploy Preview for badger-docs canceled.

Name Link
Latest commit 548c3cee3e9cae4946e90418f6160136c34ee29a
Latest deploy log https://app.netlify.com/sites/badger-docs/deploys/6571d40086bf1e0008f504fa

netlify[bot] avatar Dec 07 '23 14:12 netlify[bot]

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Dec 07 '23 14:12 CLAassistant

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Dec 07 '23 14:12 CLAassistant

This PR has been stale for 60 days and will be closed automatically in 7 days. Comment to keep it open.

github-actions[bot] avatar Jul 15 '24 01:07 github-actions[bot]

👎

dezren39 avatar Jul 16 '24 07:07 dezren39

Deploy Preview for badger-docs canceled.

Name Link
Latest commit 8c190097af363e0aec0ae089f966bca881e9bad8
Latest deploy log https://app.netlify.com/sites/badger-docs/deploys/66b23379f183a60008284c51

netlify[bot] avatar Aug 06 '24 14:08 netlify[bot]