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 AddressPool cache #195

Merged
merged 2 commits into from
Aug 21, 2014
Merged

remove AddressPool cache #195

merged 2 commits into from
Aug 21, 2014

Conversation

byxorna
Copy link
Contributor

@byxorna byxorna commented Aug 13, 2014

@maddalab @Primer42 @dallasmarlow
This PR guts the AddressPool of all IP allocation functions, and the bitset "cache" that was being used. This process-local cache has to go in order to be able to do proper loadbalancing between collins instances.

@byxorna
Copy link
Contributor Author

byxorna commented Aug 15, 2014

@maddalab @Primer42 @dallasmarlow Ive tested this via test suite, and also concurrently allocating IPs against 2 instances using the same DB to ensure no deadlocks or primary key constraints were violated, and noone attempts to allocate a duplicate IP. This seems good to go!

@yl3w
Copy link
Contributor

yl3w commented Aug 21, 2014

LGTM, did you try getting rid of the google cache library? I think this is the only use for it?

@byxorna
Copy link
Contributor Author

byxorna commented Aug 21, 2014

@maddalab the guava cache is actually a totally separate thing. The "cache" for IP address allocation in this case was actually only an in memory bit set. I looked into the guava thing but it seemed not related to ipam, and also a much bigger task. (I honestly don't fully understand what all its grafted itsself onto)

byxorna added a commit that referenced this pull request Aug 21, 2014
@byxorna byxorna merged commit 4e15fa4 into master Aug 21, 2014
@william-richard william-richard deleted the gabe-no-addresspool-cache branch September 9, 2014 19:55
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.

2 participants