New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Time slice #20

Merged

ThetaSinner merged 31 commits into main from time-slice

Nov 28, 2024

Member

ThetaSinner commented Nov 25, 2024 •

edited

Loading

Incomplete implementation of time slicing.

The pieces that are missing:

telling the time slice when the host has received new ops, so the slice hashes can be updated
providing a time slice summary to gossip and comparing with one from another peer
any logic for hash range partitioning. I see the time slicing being done per hash range, so this would need to be filtering by hash range when querying the host.

ThetaSinner commented

View reviewed changes

crates/memory/Cargo.toml Show resolved Hide resolved

ThetaSinner commented

View reviewed changes

crates/api/src/timestamp.rs Show resolved Hide resolved

ThetaSinner commented

View reviewed changes

crates/api/src/op_store.rs Show resolved Hide resolved

ThetaSinner commented

View reviewed changes

crates/api/src/op_store.rs Show resolved Hide resolved

ThetaSinner requested a review from a team

November 25, 2024 21:55

neonphog reviewed

View reviewed changes

crates/api/src/lib.rs Outdated Show resolved Hide resolved

neonphog reviewed

View reviewed changes

crates/dht/src/time.rs Show resolved Hide resolved

neonphog reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

Collaborator

neonphog commented Nov 26, 2024

Gave a cursory look over this and responded to some of your own comments. But my brain is too tired to dive in deeply tonight. I'd like to say I'll dig back into this tomorrow morning, but we've got meetings.... so after lunch?

neonphog reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

neonphog reviewed

View reviewed changes

crates/dht/src/time.rs Show resolved Hide resolved

neonphog reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

neonphog reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

neonphog reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

neonphog reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

neonphog reviewed

View reviewed changes

crates/api/src/op_store.rs Show resolved Hide resolved

Collaborator

neonphog commented Nov 26, 2024

Looks awesome! Some questions/comments, but I'm pretty happy with this : )

jost-s reviewed

View reviewed changes

Cargo.toml Show resolved Hide resolved

crates/api/src/op_store.rs Show resolved Hide resolved

crates/api/src/op_store.rs Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Show resolved Hide resolved

crates/dht/src/time.rs Show resolved Hide resolved

jost-s reviewed

View reviewed changes

Contributor

jost-s left a comment •

edited

Loading

Gonna review the tests in a bit. The code of the functions is super readable, awesome work!

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

jost-s reviewed

View reviewed changes

Contributor

jost-s left a comment

I'll continue tomorrow.

crates/memory/src/op_store.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

mattyg reviewed

View reviewed changes

crates/api/src/op_store.rs Outdated Show resolved Hide resolved

jost-s reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

crates/dht/src/time.rs Show resolved Hide resolved

crates/dht/src/time.rs Outdated Show resolved Hide resolved

mattyg reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

mattyg reviewed

View reviewed changes

crates/dht/src/time.rs Show resolved Hide resolved

mattyg reviewed

View reviewed changes

crates/dht/src/time.rs Show resolved Hide resolved

jost-s reviewed

View reviewed changes

Contributor

jost-s left a comment

Done with the first review. I appreciate the fantastic test suite!

This was referenced Nov 27, 2024

[k2] API: Host communicates op timestamp to Kitsune #31

Open

[k2] DHT: Eliminate origin_time by adding "0 run" compression #32

Open

neonphog previously approved these changes

View reviewed changes

Collaborator

neonphog left a comment

🎉

mattyg reviewed

View reviewed changes

crates/dht/src/time.rs Outdated Show resolved Hide resolved

ThetaSinner and others added 23 commits

November 28, 2024 12:17


          Fix comment

d6e7271


          Lint

842a696


          Update crates/dht/src/time.rs

d5f13d7

Co-authored-by: David Braden <[email protected]>


          Update crates/dht/src/time.rs

a835ddb

Co-authored-by: David Braden <[email protected]>


          Update crates/dht/src/time.rs

1d80690

Co-authored-by: David Braden <[email protected]>


          Update crates/dht/src/time.rs

c852e1a

Co-authored-by: David Braden <[email protected]>


          Review changes

ef2ed7a


          Update crates/dht/src/time.rs

Co-authored-by: Jost Schulte <[email protected]>


          Update crates/dht/src/time.rs

c47d672

Co-authored-by: Jost Schulte <[email protected]>


          Update crates/dht/src/time.rs

0f4a930

Co-authored-by: Jost Schulte <[email protected]>


          Return an error when the factor is invalid, rather than using a panic

4bc2cbc


          Rename from_store to try_from_store

8e35c84


          Fix adding multiple new full slices

02ef349


          Update crates/dht/src/time.rs

5d4583e

Co-authored-by: Jost Schulte <[email protected]>


          Add note about factor size limits and selection

47763b5


          Improve code breakdown so the split between time layout and hashes is…

… clearer


          Remove op flags, not relevant to this PR

4729b85


          Extract helper in tests for recent and full slice time

b3e33b8


          Update crates/dht/src/time.rs

8c9511b

Co-authored-by: Jost Schulte <[email protected]>


          Test improvements

ff1f69e


          Improve docs

eb31dee


          Add missing backtick

abc6a3a


          Update lock after rebase

2f60930

ThetaSinner force-pushed the time-slice branch from 77383d1 to 2f60930 Compare

November 28, 2024 12:17

ThetaSinner enabled auto-merge (squash)

November 28, 2024 12:18


          Format after rebase

1b1953f

jost-s approved these changes

View reviewed changes

Contributor

jost-s left a comment

🤩

ThetaSinner merged commit 335460d into main

4 checks passed

ThetaSinner deleted the time-slice branch

November 28, 2024 17:08

ThetaSinner mentioned this pull request

Two way op flow #41

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet