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

Clustering - add action for renaming replicated files #806

Closed
viktorerlingsson opened this issue Oct 15, 2024 · 4 comments · Fixed by #812
Closed

Clustering - add action for renaming replicated files #806

viktorerlingsson opened this issue Oct 15, 2024 · 4 comments · Fixed by #812
Assignees

Comments

@viktorerlingsson
Copy link
Member

viktorerlingsson commented Oct 15, 2024

Add a RenameAction to clustering.

Needed for #661

We probably need to add a more explicit way to select what action to be taken.
Now it's determined by an Int64 that also holds the length of the content to sync: delete file if it's 0, append to file if it's negative and add file if it's positive.

So maybe add an Int8 for action selection.

@viktorerlingsson viktorerlingsson self-assigned this Oct 15, 2024
@carlhoerberg
Copy link
Member

Why is it necessary? If you receive a file you already got, then store it to a temp file and then rename it back when you received the full file.

@viktorerlingsson
Copy link
Member Author

Hmm, yeah I guess that would work just as well! Maybe it would be a little more explicit with a separate action for renaming, but there shouldn't be much of a difference in reality.

What do you think about adding an extra byte for action? Could be pretty useful if we want to be able to perform more actions on a follower, like a health check or so. But probably not worth looking into until we actually need it!

@carlhoerberg
Copy link
Member

I would like to avoid it. Less data = less bandwidth and less cpu cycles for encoding/decoding. Would also break protocol compatibility.

@viktorerlingsson
Copy link
Member Author

I would like to avoid it. Less data = less bandwidth and less cpu cycles for encoding/decoding.

Understood. Yeah, there would be an overhead, but not that much? A message with a one character payload is like 138 bytes to sync already, so I'm thinking one extra byte wouldn't make that big of a difference. Not sure how much extra that would add in encoding/decoding though.
(For delete I think it could actually make the payload 7 bytes smaller, as we wouldn't need to send the 0i64 for size. But deletes should be much rarer than appends.)

Would also break protocol compatibility.

Yes, but before 2.0 release is probably a good time to do that if we ever are going to break it 🙂 Maybe we could also use part of the byte for a "protocol version" for future proofing.

But again, probably not worth doing until we actually need more actions!

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 a pull request may close this issue.

2 participants