Skip to content
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.

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

Make server logic vector clock-aware #153

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

richardhuaaa
Copy link
Contributor

@richardhuaaa richardhuaaa commented Sep 10, 2024

I was worried about type-conversions between int32 and uint32 and overflow issues, but it looks like with two's complement, overflowed int32's behave the same as uint32's after conversion (or in other words, the increment operation does exactly the same thing to the underlying byte representation either way). In other words, our postgres schema can only support signed int32's, but it seems like we can just pretend they are uint32's and have the same effect.

We do still need to worry about the postgres sequence overflowing, as well as the max value of uint32/uint64 - when implementing the node-to-node syncing logic, my plan is to regularly log the node's vector clock, so that we can see it coming from a long way away.

Closes #132

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @richardhuaaa and the rest of your teammates on Graphite Graphite

Copy link

graphite-app bot commented Sep 10, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “Queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “Hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Comment on lines +11 to +15
type PollableDBQuery[ValueType any, CursorType any] func(
ctx context.Context,
lastSeen CursorType,
numRows int32,
) (results []ValueType, nextCursor CursorType, err error)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main change is here - PollableDBQuery now uses a generic cursor type rather than an int64

@@ -16,7 +16,8 @@ import (
)

const (
maxRequestedRows int32 = 1000
maxRequestedRows uint32 = 1000
maxVectorClockLength int = 100
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this mainly to stop malicious actors from flooding our DB with large requests using huge vector clock queries. We could theoretically make this bigger

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, this should always be < the number of nodes in the registry. Maybe that can be the upper bound?

@richardhuaaa richardhuaaa marked this pull request as ready for review September 10, 2024 21:26
@richardhuaaa richardhuaaa requested a review from a team as a code owner September 10, 2024 21:26
@richardhuaaa richardhuaaa enabled auto-merge (squash) September 11, 2024 18:01
@richardhuaaa richardhuaaa merged commit 434dc51 into main Sep 11, 2024
5 checks passed
@richardhuaaa richardhuaaa deleted the 09-09-make_server_logic_vector_clock-aware branch September 11, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cursor/query for clients that switch gateway nodes
2 participants