feat: add peer recon ring #616
Conversation
nathanielc
requested review from
stephhuynh18
and removed request for
a team
| .bind(range.start.as_bytes()) | ||
| .bind(range.end.as_bytes()) | ||
| .bind(limit as i64) | ||
| .bind(offset as i64) |
There was a problem hiding this comment.
I don't think this cast is okay. You pass in usize::MAX so it should probably be try_into().unwrap_or(i64::MAX)
There was a problem hiding this comment.
Hmm, good catch, however I don't think we can just use i64::MAX safely either. If we change the limit or offset values the caller passes in they will not be able to page through values. It appears sqlite does not support u64 so I think we need to use u32 for all limit offset pagination indexes we use. This seems sane as a limit or offset larger than u32 is quite large. I'll do this is a seperate PR.
There was a problem hiding this comment.
Well u32 won't work its too small to be able to count the data. So either we need to use i64 (aka u63) or use Option for limit offset and handle it explicitly in queries. I think we should use i64. Thoughts?
There was a problem hiding this comment.
hmm. yeah, I think i64 is fine. it'd probably be nice to use cursor style instead of offset/limit but we generally use offset=0 so I think it's pretty much just cursor+limit in practice.
which makes me question if we actually need these parameters.. could we use the range fenceposts? it seems like we should be able to paginate with either left+limit, or a range where we want everything inside it. I might be forgetting why we need this though.
| Error, Result, | ||
| }; | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq)] |
There was a problem hiding this comment.
I think these files (access/version.rs and entities/version.rs) can be deleted from the peers-svc
| } | ||
|
|
||
| /// Return the hash of all keys in the range between left_fencepost and right_fencepost. | ||
| /// Both range bounds are exclusive. |
There was a problem hiding this comment.
should the exclusive comments in this file actually be left inclusive? the queries appear to be left inclusive
nathanielc
force-pushed
the
feat/peer-recon-ring
branch
from
37cedcd
to
5b74e9d
Compare
nathanielc
force-pushed
the
feat/peer-recon-ring
branch
from
5b74e9d
to
9e8d80e
Compare
nathanielc
temporarily deployed
to
dev-qa-2024
GitHub Actions
Inactive
nathanielc
temporarily deployed
to
dev-qa-2024
GitHub Actions
Inactive
nathanielc
temporarily deployed
to
dev-qa-2024
GitHub Actions
Inactive
With this change there is now a Recon ring for sharing information about
peers. Peers will synchronized all known peers with connected peers.
Each peer will self-publish into the ring periodically.
Fixes: #604
Fixes: #607