Sled: A shared trate for Tree and TransactionTree

Created on 11 Aug 2020  路  3Comments  路  Source: spacejam/sled

Use Case:

It would be useful to be able to write functions that abstract common database tasks in my application that work in both transactional and non-transactional contexts. At the moment each utility functions can only either accept a Tree or a TransactionTree from Sled. To write functions generic to transactional and non-transactional code we'd need a Trait that covers both.

Proposed Change:

Add a trait that covers the common capabilities of Tree and TransactionTree taking the UnabortableTransactionError error signatures from TransactionTree.

Who Benefits From The Change(s)?

Me! :)

More generally: anyone who wishes to build their own internal utilities or crates across Sled transaction + non-transactional contexts.

Alternative Approaches

Duplicate each function for transactional/non-transactional contexts. I leave this open to better suggestion.

feature

Most helpful comment

I don't want users to ever have to import a trait before core functionality is supported, but I also want to make things easier. I think the best transactional API will be to actually remove TransactionalTree (and the Transactional trait) entirely and instead support just using normal Trees inside a function like this, which sets a thread-local variable (and breaks Send, making it safe for non-multithreaded async schedulers):

let db = sled::open("db")?;
sled::transaction(|| {
  db.set(b"k1", b"v1")?;
});

This is my current line of thinking and in the process it will hopefully solve multiple goals of reducing complexity, reducing the amount of typing anyone has to do to use basic functionality, avoiding things breaking because some trait wasn't used accidentally, and generally opening up all Tree functions for use within a transactional context.

All 3 comments

I don't want users to ever have to import a trait before core functionality is supported, but I also want to make things easier. I think the best transactional API will be to actually remove TransactionalTree (and the Transactional trait) entirely and instead support just using normal Trees inside a function like this, which sets a thread-local variable (and breaks Send, making it safe for non-multithreaded async schedulers):

let db = sled::open("db")?;
sled::transaction(|| {
  db.set(b"k1", b"v1")?;
});

This is my current line of thinking and in the process it will hopefully solve multiple goals of reducing complexity, reducing the amount of typing anyone has to do to use basic functionality, avoiding things breaking because some trait wasn't used accidentally, and generally opening up all Tree functions for use within a transactional context.

@spacejam That sounds ideal. If I understand it right we're using internal mutability to mark a transaction as open in your example?

If were moving in that direction I'd really like to see the ConflictableTransactionError replaced with internal mutability as well (eg. with a should_retry_transaction flag to mark conflicts) so that errors from inside a transaction can be passed through to the db.transaction caller transparently.

The current error wrapping solution suffers from difficult ergonomics when combined with non std::Error results such as anyhow.

I made an initial attempt at anyhow compatibility on this branch but having gone down that path I'm dissatisfied with my solution and I'd much prefer internal mutability because it should allow for things like backtrace to work without special hacks.

I'd be interested in taking a shot at that change myself if it sounds like a potential direction you'd like to go with sled.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

thedrow picture thedrow  路  7Comments

spacejam picture spacejam  路  4Comments

spacejam picture spacejam  路  6Comments

spacejam picture spacejam  路  7Comments

ckaran picture ckaran  路  6Comments