feat: add value support to Recon #217
Conversation
| pub first: K, | ||
| /// Hash of all keys in between first and last exclusive. | ||
| /// Hash is zero if there are no keys within the range. | ||
| pub hash: HashCount<H>, |
There was a problem hiding this comment.
Should this be an Optional HashCount
core::RangeOpen and recon:Range feel like they are similar do we want difrent types for ranges based on wether the hash and count are known?
Obviously we need the hash if we want to send it across the wire.
Maybe hash_range should take a RangeOpen and return a Range?
There was a problem hiding this comment.
I actually started with an Option but it turns out since Hash has a well defined zero value that always expecting the hash was better. This way sqlite computing a sum doesn't have to have logic to check for a zero and return a None instead. In other words there is no meaningful difference between a None hash and a zero hash so an Option only adds unneeded complexity.
Having two types works well for me. However we could maybe have better names? And yes a nice refactor later would be to change all the method that accept left_fencepost and right_fencepost to simply accept a RangeOpen.
There was a problem hiding this comment.
The None would be unknown not zero. But look at it the second time having one type for an open range and different one for a range with its hash and count is clearer. There is no one place where we have ranges but don't know if we retrieved the hash and count yet.
nathanielc
force-pushed
the
feat/recon-values
branch
from
f9033e1 to
c8b8902
Compare
recon/src/recon/sqlitestore.rs
Outdated
| .bind(&self.sort_key) | ||
| .bind(key.as_bytes()) | ||
| .fetch_all(self.pool.writer()) | ||
| .await; |
There was a problem hiding this comment.
Is it appropriate to use ? here?
There was a problem hiding this comment.
Yep that would simplify, will make the change.
With this change Recon is a key value store instead of simply a key store. Recon synchronization is the same in principle but its underlying protocol has been refactored. The protocol design is now has clear initiator and responder roles. Generally the initiator makes requests and the responder responds. Messages themselves are small and represent a small amount of work. This way both nodes can be actively working concurrently to synchronize. Prior to this change nodes would be idle while the other was working and would frequently deadlock if a single message size grew too large. Values are synchronized by sending ValueRequests and ValueResponses along side synchronization messages. The API into the Recon protocol is now a Stream + Sink allowing any system that can transport messages in full duplex a possible transport for Recon. The protocol implementation is no longer specific to libp2p. This means we can use HTTP in the future. However this PR removes the HTTP implementation as its not trivial with the HTTP server we are using to create a full duplex channel.
nathanielc
force-pushed
the
feat/recon-values
branch
from
c3a1d88 to
7f3c1c9
Compare
With this change Recon is a key value store instead of simply a key store.
Recon synchronization is the same in principle but its underlying protocol has been refactored.
The protocol design is now has clear initiator and responder roles. Generally the initiator makes requests and the responder responds. Messages themselves are small and represent a small amount of work. This way both nodes can be actively working concurrently to synchronize. Prior to this change nodes would be idle while the other was working and would frequently deadlock if a single message size grew too large.
Values are synchronized by sending ValueRequests and ValueResponses along side synchronization messages.
The API into the Recon protocol is now a Stream + Sink allowing any system that can transport messages in full duplex a possible transport for Recon. The protocol implementation is no longer specific to libp2p. This means we can use HTTP in the future. However this PR removes the HTTP implementation as its not trivial with the HTTP server we are using to create a full duplex channel.