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

clientv3: fix the implementation of double barrier #14604

Merged
merged 1 commit into from
Oct 31, 2022

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Oct 20, 2022

Fix #14603

Check the waiter count before creating the ephemeral key, do not create the key if there are already too many clients. Check the count after creating the key again, if the total kvs is bigger than the expected count, then check the rev of the current key, and take action accordingly based on its rev. If its rev is in the first ${count}, then it's valid client, otherwise, it should fail.

Signed-off-by: Benjamin Wang wachao@vmware.com

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

Check the client count before creating the ephemeral key, do not
create the key if there are already too many clients. Check the
count after creating the key again, if the total kvs is bigger
than the expected count, then check the rev of the current key,
and take action accordingly based on its rev. If its rev is in
the first ${count}, then it's valid client, otherwise, it should
fail.

Signed-off-by: Benjamin Wang <wachao@vmware.com>
@ahrtr
Copy link
Member Author

ahrtr commented Oct 23, 2022

Please note that the logic is similar to the double check mechanism (check --> register --> check) used in v3discovery.

If there are multiple clients register concurrently, then it's possible the registered client count is greater than the expected count (assuming N). In this case, we just sort all the client by createRevision, and the first N clients are valid, and all others are invalid.

cc @mitake @serathius @spzala @ptabor

@ahrtr ahrtr changed the title clientv3: fix the design & implementation of double barrier clientv3: fix the implementation of double barrier Oct 25, 2022
@ahrtr
Copy link
Member Author

ahrtr commented Oct 27, 2022

This PR should can fix issue #14603, please also take a look, thx. cc @serathius @spzala @ptabor @mitake

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @ahrtr

@ahrtr
Copy link
Member Author

ahrtr commented Oct 30, 2022

Thanks @spzala

@ahrtr ahrtr merged commit a1018db into etcd-io:main Oct 31, 2022
@serathius serathius mentioned this pull request Nov 14, 2022
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

runtime: goroutine stack exceeds 1000000000-byte limit
2 participants