node-replication icon indicating copy to clipboard operation
node-replication copied to clipboard

Nr dynamic replication

Open MarcPaquette opened this issue 2 years ago • 2 comments

This addresses issue 52.

We've added add_replica and remove_replica to the NodeReplicated. Thread context is held within the replica . The number of new replicas are limited via MAX_REPLICAS_PER_LOG.

MarcPaquette avatar Feb 03 '23 18:02 MarcPaquette

Had a first look, and it looks great!

I think there are some minor things to iron out:

  • I pushed a small multi-threaded example to test it, and it currently fails which is due to missing the re-routing for threads to the right replicas (in case a thread is already registered to a replica that got to be removed):
$ cargo run --example nr_dynamic_replica
   Compiling node-replication v0.2.0 (/home/gz/node-replication/node-replication)
    Finished dev [unoptimized + debuginfo] target(s) in 0.68s
     Running `target/debug/examples/nr_dynamic_replica`
About to remove replica 3
Removed replica 3
thread '<unnamed>' panicked at 'no entry found for key', /home/gz/node-replication/node-replication/src/nr/mod.rs:594:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'thread 'thread '<unnamed><unnamed><unnamed>' panicked at '' panicked at '' panicked at 'no entry found for keyno entry found for keyno entry found for key', ', ', /home/gz/node-replication/node-replication/src/nr/mod.rs/home/gz/node-replication/node-replication/src/nr/mod.rs/home/gz/node-replication/node-replication/src/nr/mod.rs:::681694694:::292121
  • OTOH seems like add_replicas is doing the right thing from what I can tell :tada:

  • The return value for add_replica is ThreadToken but maybe it should just be the replica id of the added replica?

  • Another thing we need to think about is that add_replica and remove_replica currently require &mut self. This is fine for now as we can just wrap it in a n RwLock for testing correctness but we probably want these to just take &self in the future.

gz avatar Feb 03 '23 21:02 gz

Thanks for the comments. I'll work on addressing your feedback!

MarcPaquette avatar Feb 03 '23 22:02 MarcPaquette