perf: [NPM] make apply ipsets faster #1307
Merged
+1,327
−255
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
huntergregory
force-pushed
the
npm-logs-and-ipset-restore-speed
branch
3 times, most recently
from
23f8c0b to
b95cf6e
Compare
vakalapa
requested changes
Apr 7, 2022
huntergregory
force-pushed
the
npm-logs-and-ipset-restore-speed
branch
from
0dd7e58 to
bab47ba
Compare
vakalapa
reviewed
Apr 13, 2022
matmerr
reviewed
Apr 14, 2022
|
|
||
| // NOTE: duplicate code in the first step in this function and fileCreatorForApplyWithSaveFile | ||
| func (iMgr *IPSetManager) fileCreatorForApply(maxTryCount int) *ioutil.FileCreator { | ||
| creator := ioutil.NewFileCreator(iMgr.ioShim, maxTryCount, ipsetRestoreLineFailurePattern) // TODO make the line failure pattern into a definition constant eventually |
Member
There was a problem hiding this comment.
should we persist these creators within the ipsetmanager to avoid creating these in both this and fileCreatorForApplyWithSaveFile?
Contributor
Author
There was a problem hiding this comment.
talked offline. rough to duplicate a lot between the functions but no clean solution
vakalapa
approved these changes
Apr 18, 2022
Contributor
vakalapa
left a comment
vakalapa
left a comment
There was a problem hiding this comment.
Great work, this improves our perf by about 10x !! 
matmerr
pushed a commit
to matmerr/azure-container-networking
that referenced
this pull request
Jun 29, 2022
* wip * fix member update * fix hashed name bug * fix hashed name bug and bug for original members for delete cache * fix delete bug * print cache in logs again * some UTs * dirty cache UTs * fix windows build problem * dirty cache with members to add/delete & keep old apply with save file code * fix windows * fix windows 2 * fix windows 3 * UTs and dirty cache in same struct shared between each OS * fix windows build * change some error situations to valid situations * log clean up and addressing comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
dramatically improves apply time for medium to large scale.
Previously, we would parse through an ipset save file for every batch dataplane operation. We can forego that by maintaining member diff for our ipset dirty cache.
New dirty cache adds robustness to all combinations of create/add/del/destroy calls. Windows doesn't need to maintain member diff by nature of HNS.