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

Remove VectorClock type #235

Merged
merged 5 commits into from
Jan 7, 2025
Merged

Remove VectorClock type #235

merged 5 commits into from
Jan 7, 2025

Conversation

richardhuaaa
Copy link
Contributor

We are no longer referring to this as a 'vector clock', instead it could be more accurately called a cursor.

Seeing as we are renaming anyway, I thought it might be a good opportunity to unnest it, as it complicated the code for using the field. In the original layout, the field could be referred to by three nested names: last_seen, VectorClock, node_id_to_sequence_id.

The issue though is that this is a breaking change, and would require resetting the DB. We may have to do this not just for our own instance of the node, but in any other partner nodes. If this ends up being too difficult, I'm happy to avoid the unnesting and simply rename VectorClock->Cursor. Although it could be a good thing to run through a breaking change as practice regardless.

@richardhuaaa richardhuaaa requested a review from a team December 18, 2024 22:45
@richardhuaaa richardhuaaa requested a review from a team as a code owner December 18, 2024 22:45
@@ -4,16 +4,15 @@ package xmtp.identity.api.v1;

import "google/api/annotations.proto";
import "identity/associations/association.proto";
import "identity/associations/signature.proto";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated lint/auto-format changes

Copy link
Collaborator

@neekolas neekolas left a comment

Choose a reason for hiding this comment

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

I think we should do a trial run of a breaking change.

I don't think the week before Christmas is the week to do it. Very soon it's going to be hard to get a hold of node operators. If you want this update before January, I'd suggest doing it in the non-breaking way.

@richardhuaaa
Copy link
Contributor Author

There is no rush on this - just put this up so that we have it on our minds. Can wait until January

@richardhuaaa
Copy link
Contributor Author

Okay, I figure it is easier to separate the breaking change to a separate PR if we really need it. Just renaming for now which can unblock us on more short-term things like releasing the XIP

@richardhuaaa richardhuaaa requested review from neekolas and a team January 3, 2025 02:06
@richardhuaaa richardhuaaa merged commit e295b46 into main Jan 7, 2025
4 checks passed
@richardhuaaa richardhuaaa deleted the rich/remove-vector-clock branch January 7, 2025 22:58
Copy link

🎉 This PR is included in version 3.73.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants