Way to combine multiple `concread` structures?

[Erigara]

Hello, not sure it's the best place to ask questions, but hope it's okay.

I want to combine multiple concread structures together while still preserve transactional behavior.

It looks like that without additional coordination it's possible reader could get inconsistent results where fields were partially committed.

struct State {
     a: concread::Bptree<K1, V1>,
     b: concread::Bptree<K2, V2>,
}

struct StateReadTx<'a> {
    a: concread::BptreeMapReadTxn<'a, K1, V1>,
    b: concread::BptreeMapReadTxn<'a, K2, V2>,
}

struct StateWriteTx<'a>
    a: concread::BptreeMapWriteTxn<'a, K1, V1>,
    b: concread::BptreeMapWriteTxn<'a, K2, V2>,
}

So solution i came up so far is inclusion of additional Mutex which will be locked during commit and creation of read transaction (write transactions are already protected by writer lock).

struct State {
    // ---//---
    lock: Mutex<()>,
}

struct StateWriteTx<'a>
    a: concread::BptreeMapWriteTxn<'a, K1, V1>,
    b: concread::BptreeMapWriteTxn<'a, K2, V2>,
    lock: &'a Mutex<()>,
}

impl State {
    fn read(&self) -> StateReadTx<'_> {
        let _lock = self.lock.lock().unwrap();
        StateReadTx { a: self.a.read(), b: self.b.read() }
    }
    
    fn write(&self) -> StateWriteTx<'_> {
        StateWriteTx { a: self.a.write(), b: self.b.write(), lock: &self.lock }
    }
}

impl StateWriteTx<'_> {
    fn commit(self) {
       // Enter critical section to prevent inconsistent reader transactions
       let _lock = self.lock.lock().unwrap();
       self.a.commit();
       self.b.commit();
    }
}

However this solution suffers from that now write transaction commit is blocked by reader creation.
Is there is something can be done about this? Maybe someone has already solved such problem.

My another idea is to use approach similar to SeqLock where reader will retry in a loop until get consistent result, i expect number of retries to be low due to infrequent writes (about once per second).

[Firstyear]

I'm not sure that you will end up with partial results? If you take writes in the order A, B, then when you commit you commit B, A. Because you unwind in the opposite direction, A is protecting the whole operation so a subsequent reader can't take B until A finalises. That should remove the need for your lock here.

By the same rule, if you take reads in the same order, you'll have no issues. You don't need to worry about reader drop order.

Additionally we have looked into making a multi-tree coordinator, but it's still in development.

[Erigara]

Thanks for the answer, this indeed works.
Feel free to close.

[Firstyear]

No problem, glad I could help.

[Erigara]

Hey, maybe i did smt wrong but this program fails:

use std::{thread, sync::Arc};

use concread::bptree::{BptreeMap, BptreeMapWriteTxn, BptreeMapReadTxn};

struct State {
    a: BptreeMap<(), u64>,
    b: BptreeMap<(), u64>,
}

struct StateWrite<'state> {
    a: BptreeMapWriteTxn<'state, (), u64>,
    b: BptreeMapWriteTxn<'state, (), u64>,
}

struct StateRead<'state> {
    a: BptreeMapReadTxn<'state, (), u64>,
    b: BptreeMapReadTxn<'state, (), u64>,
}

impl State {
    fn new() -> Self {
        Self {
            a: BptreeMap::from_iter([((), 0)]),
            b: BptreeMap::from_iter([((), 0)]),
        }
    }

    fn read(&self) -> StateRead<'_> {
        // also tried to take fields in b,a order
        StateRead {
            a: self.a.read(),
            b: self.b.read(),
        }
    }

    fn write(&self) -> StateWrite<'_> {
        StateWrite {
            a: self.a.write(),
            b: self.b.write(),
        }
    }
}

impl StateWrite<'_> {
    fn set_and_commit(mut self, v: u64) {
        self.a.insert((), v);
        self.b.insert((), v);
        self.b.commit();
        self.a.commit();
    }
}

fn main() {
    let state = Arc::new(State::new());
    let state_clone = state.clone();
    let read = thread::spawn(move || {
        loop {
            let read = state_clone.read();
            let a = read.a.get(&());
            let b = read.b.get(&());
            if a >= Some(&10_000) {
                break;
            }
            assert_eq!(a, b);
        }
    });
    let write = thread::spawn(move || {
        for i in 0..10_000 {
            state.write().set_and_commit(i);
        }
    });
    write.join().unwrap();
    read.join().unwrap();
}

[Firstyear]

I'm thinking that there may be a race between your two read calls. My thinking is you have;

|    t1       |       t2       |

 write a
 write b
                read a
 commit b
                read b
 commit a

My thinking is you may need to test with a pattern like https://github.com/kanidm/concread/blob/master/src/lc_tests.rs which protects a and b together.

I have struggled a bit to find a "nice way" to wrap this for usage externally which is why this stalled somewhat here. But ultimately I think it's what you want, and it has the nice effect of reducing the arcs/mutexes to absolute minimum since they're shared behind a single linear cow cell. If you have some ideas to make a nicer interface for this, I'm very open to ideas!

[Firstyear]

And given you already have looked at the code, I'm sure you already know that https://github.com/kanidm/concread/blob/master/src/bptree/impl.rs#L315 is just a thin wrapper over cursor anyway so it's not going to affect functionality.