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

Adding V6 support to svcwatcher, plus fixing concurrent updates #187

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

Levovar
Copy link
Collaborator

@Levovar Levovar commented Mar 6, 2020

DANM svcwatcher managed EndPoints now support IPv6 addresses as well!

also correcting #185
Correction is not the elegant one, but should work with the current architectural constraints.
We will infinitely re-try EndPoint update in case of conflict, but only retry for 1 sec in case of temporary network outages etc.

@Levovar Levovar linked an issue Mar 9, 2020 that may be closed by this pull request
@Levovar Levovar force-pushed the svcw_update_fix branch 2 times, most recently from 817c6a5 to 85419fa Compare March 17, 2020 15:50
Also adding solution to handle conflicting endpoint update scenarios.
Correction is not the elegant one, but should work with the current architectural constraints.
We will infinitely re-try EndPoint update in case of conflict, but only retry for 1 sec in case of temporary network outages etc.
…failure during creation

Needed to rework concurrent update handling, because it is not enough to retry the operation,
we also need to reapply the changes to the new EP structure.
In every block where EP is updated.

Lastly, found a random core in the new DeleteDanmEp API.
@Levovar
Copy link
Collaborator Author

Levovar commented Mar 19, 2020

ok, so to record for the next generation: I ended up adding an extremely ugly concurrency handling block to all EndPoint update part
it looks to be working, but I can't stand to look at it. anyone feeling frisky enough has my blessing to refactor the whole controller :)

other than that, performance bottlenecks can happen if the same EndPoint in the same service needs to be conccurently updated, e.g. you create 100 Pods at the same time, matching the exact same Service
we will probably handle the state change flood eventually, but it will take some seconds to correctly update all PodIPs
IMO it should work okayish until like 100 concurrent updates, wouldn't try using it for thousand Pods / Service

@Levovar Levovar merged commit fd4dd71 into master Mar 19, 2020
@Levovar Levovar deleted the svcw_update_fix branch March 19, 2020 13:11
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.

Svcwatcher doesn't handle concurrent updates correctly
1 participant