-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
ldap: add timeout mechanism and optimize lock for LDAP dial and requests #51912
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #51912 +/- ##
================================================
+ Coverage 72.1416% 75.0818% +2.9402%
================================================
Files 1467 1489 +22
Lines 426665 435562 +8897
================================================
+ Hits 307803 327028 +19225
+ Misses 99697 87623 -12074
- Partials 19165 20911 +1746
Flags with carried forward coverage won't be shown. Click here to find out more.
|
fbe1b04
to
af28965
Compare
/retest |
Signed-off-by: Yang Keao <[email protected]>
Signed-off-by: Yang Keao <[email protected]>
7c76868
to
ffa01ca
Compare
Now, I think this PR is ready to merge. cc @bb7133 and @CbcWestwolf |
/hold I found a bug. If |
What problem does this PR solve?
Issue Number: close #51883
Problem Summary:
There are two problems:
rebuildSysVarCache
.What changed and how does it work?
I have done two modification to fix this problem:
ldapAuthImpl
toldapAuthImplBuilder
, which will copy the configurations (and take a reference to the connection pool, which include a closure with a copy of connection configurations). Therefore, during the scope of any lock, there didn't exist any IO operation, so that therebuildSysVarCache
will not be blocked by a network connection.Check List
Tests
I used the
yangkeao/ldap-sasl-example
container to run the manual test, steps:TODO
Release note