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

client version in cluster state dumps. #312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GaryWKeim
Copy link
Contributor

@GaryWKeim GaryWKeim commented Jun 25, 2019

Copy link
Member

@anthonydahanne anthonydahanne left a comment

Choose a reason for hiding this comment

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

Thanks to this change, we'll be able to track clients version using the MnM stack; since the serial ID did not change, I'll make sure other PRs are version proof catching proper exceptions when deserializing

@anthonydahanne
Copy link
Member

@myronkscott can you please add your review ? thanks

@myronkscott
Copy link
Member

I have a concern that in rare cases, this change may crash the cluster. If a stripe split-brains, then heals, the active will as the other active for information that includes this serialized object. I know it is not a great solution, but maybe we should include version information as part of the UUID.

@myronkscott
Copy link
Member

I think the best way to handle this is to add a new sub-class of PlatformConnectedClient which includes the version.

Copy link
Member

@myronkscott myronkscott 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 need a new subclass rather than modifying the existing one.

@anthonydahanne
Copy link
Member

hello, @myronkscott do you want me or @GaryWKeim to add this new subclass ?

@myronkscott
Copy link
Member

hello, @myronkscott do you want me or @GaryWKeim to add this new subclass ?

I think we need to discuss. Even a subclass is bad as a legacy server will not have the sub-class and cause problems.

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.

3 participants