-
Notifications
You must be signed in to change notification settings - Fork 260
Optimize call flow to complete loopback adapter creation #408
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
Optimize call flow to complete loopback adapter creation #408
Conversation
Codecov Report
@@ Coverage Diff @@
## master #408 +/- ##
=======================================
Coverage 52.29% 52.29%
=======================================
Files 28 28
Lines 4044 4044
=======================================
Hits 2115 2115
Misses 1654 1654
Partials 275 275Continue to review full report at Codecov.
|
| bytes, err := c.Output() | ||
| loopbackOperationLock.Unlock() | ||
|
|
||
| if err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think we can just combine this? a single line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which lines are you suggesting to combine?
| } | ||
|
|
||
| func setWeakHostOnInterface(ipAddress string) error { | ||
| func setWeakHostOnInterface(ipAddress, ncID string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this function even needed now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean separately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there way to asset the lock has been acquired already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asset the lock?
Keeping the function separate because it's still a separate functionality
| } | ||
|
|
||
| func setWeakHostOnInterface(ipAddress string) error { | ||
| func setWeakHostOnInterface(ipAddress, ncID string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there way to asset the lock has been acquired already?
Loopback adapter creation operation comprises of two operations - createInterface and setWeakHostOnInterface. These operations take place inside the lock. If there are simultaneous requests, it interleaves these calls causing every loopback adapter creation to absorb the delay due to interleaving. createInterface can take time in seconds (typically 2 to 7 seconds based on the tests) while setWeakHostOnInterface finishes very quickly ( less than a second ). This change calls setWeakHostOnInterface within the same lock if createInterface succeeds. The tests show this improves the loopback adapter creation times for simultaneous requests.
What this PR does / why we need it:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Release note: