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

Leaked write transactions are hard to avoid #320

Closed
casey opened this issue Aug 6, 2022 · 8 comments · Fixed by #321
Closed

Leaked write transactions are hard to avoid #320

casey opened this issue Aug 6, 2022 · 8 comments · Fixed by #321

Comments

@casey
Copy link
Contributor

casey commented Aug 6, 2022

Idiomatic code tends to leak write transactions. We have a few places in ord where we're doing something like this:

let tx = database.begin_write()?;
let value = might_fail(tx)?;
// do more stuff with `value`
tx.commit()?;

If might_fail returns Result::Err, then we'll leak the write transaction, and get the message about leaking write transactions.

To work around this and avoid getting the error message, we have to introduce wrappers like this:

let tx = database.begin_write()?;
let value match might_fail(tx) {
  Ok(value) => value,
  Err(err) => {
    tx.abort()?;
    return Err(err);
  }
}
// do more stuff with value
tx.commit()?;

Also, sometimes there isn't just one function might_fail, there are a few in sequence, and either each one needs a wrapper, or the sequence needs to be extracted into a sub-function, so that you can match sub_function(tx). Both options are pretty verbose.

Given that, perhaps write transactions should just abort on drop?

@casey
Copy link
Contributor Author

casey commented Aug 6, 2022

Aborting on drop would swallow errors, since Drop::drop returns (). However, there's some precedence for this in the standard library. std::fs::File::drop flushes the file, and ignores any errors that occur there.

@cberner
Copy link
Owner

cberner commented Aug 6, 2022

Ah yes, I like that better. I avoided automatically aborting since it would ignore the errors, but if std does it 🤷

@casey
Copy link
Contributor Author

casey commented Aug 6, 2022

Sweet, this is great! I can remove my weird AbortOnDrop wrapper now.

@cberner
Copy link
Owner

cberner commented Aug 7, 2022

Just released this in 0.5.0

@casey
Copy link
Contributor Author

casey commented Aug 7, 2022

Sweet, we'll update soon.

The ordinal explorer is getting more featureful by the way:

https://signet.ordinals.com/

The front page returns a list containing every signet block, which requires iterating over a 100k entry table, and redb is super fast.

The only reason we're doing something so stupid is because we haven't implemented pagination, but it's nice that we don't need to implement pagination just because our database is slow. (The reason we have to implement pagination is because it's not polite to return 16 MiB of HTML unless you're the whatwg.)

@casey
Copy link
Contributor Author

casey commented Aug 7, 2022

You can also look up ordinals now by name, number, degree, or decimal: https://signet.ordinals.com/ordinal/cberner

@cberner
Copy link
Owner

cberner commented Aug 7, 2022

Nice! How is the name calculated?

@casey
Copy link
Contributor Author

casey commented Aug 7, 2022

It's close to being a straight conversion from base 10 / 0-9 to base 26 / a-z. However it goes from z to aa and not from z to ba, which is what you would get if a was 0 and b was one.

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 a pull request may close this issue.

2 participants