Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

For discussion - deloop dispatch #192

Closed
wants to merge 4 commits into from
Closed

For discussion - deloop dispatch #192

wants to merge 4 commits into from

Conversation

aboodman
Copy link
Contributor

This PR doesn't work but shows my train of thought.

We change dispatch into a "normal" function that delegates directly to the "doer" functions. I already tested (see do_test) that the basic concept works -- the futures get executed and calls can overlap this way.

However, I rediscovered why we need the dispatch loop!

It's fundamentally because of the relationship between transactions and stores. Transactions have a reference to their store, so the store must outlive the transaction. In particular the RWLockGuard in kv::{Read,Write}Transaction.

My thought was that the RWLock would let us get around this, but it does not. What it weakens is the single writer requirement. The reference requirement remains.

The openTransaction RPC fundamentally cannot work this way due to
the lifetime parameters in kv::Store::read() and write(): You can't
hold a guard beyond a stack frame because it needs to live as long
as the store that creates it and there's no way to statically
prove that to the compiler.

I can think of a few paths for exploration:

1. Have the transactions refcount the store.
2. Have the store own the transactions. This makes a lot of sense
   conceptually I think, but I think it would require big changes
   to dag::Read/Write and db::Read/Write, which are designed to
   wrap/own the lower level stores.
Copy link
Contributor

@phritz phritz left a comment

Choose a reason for hiding this comment

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

Thanks for sharing, not surprised there are lifetime issues. I'm not going to spend time grokking this fully tho unless you want me to.

@aboodman aboodman closed this Sep 29, 2020
@aboodman aboodman deleted the deloop branch April 8, 2021 20:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants