-
Notifications
You must be signed in to change notification settings - Fork 4.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
lock names: Remove uid suffix & hash entire path #6059
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tstromberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/ok-to-test |
Error: running mkcmp: exit status 1 |
Codecov Report
@@ Coverage Diff @@
## master #6059 +/- ##
=========================================
- Coverage 36.74% 36.7% -0.04%
=========================================
Files 113 113
Lines 8366 8355 -11
=========================================
- Hits 3074 3067 -7
+ Misses 4890 4887 -3
+ Partials 402 401 -1
|
All Times minikube: [ 127.604308 130.908744 127.251100] Average minikube: 128.588051 Averages Time Per Log
|
All Times minikube: [ 126.190235 126.979790 128.087883] Average minikube: 127.085969 Averages Time Per Log
|
maybe add a description to the PR that why you think it fixes the issue ? |
ok to test |
… filenames rather than descriptive names
All Times minikube: [ 129.505926 126.133423 113.651865] Average minikube: 123.097071 Averages Time Per Log
|
All Times minikube: [ 132.483634 125.859874 127.080990] Average minikube: 128.474833 Averages Time Per Log
|
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 like this a lot more conceptually.
This PR makes lock names unique again for Windows users.
Currently, we generate lock names by taking the last 39 characters of:
fmt.Sprintf("%s-%s", path, uid))
On most platforms, uid's are 0-6 characters long. However, on Windows, a SID is returned, which looks like:
S-1-5-21-1180699209-877415012-3182924384-1004
That's 46 characters long. Effectively, on Windows, all of our locks shared the same name:
m21-1180699209-877415012-3182924384
Why was the UID added anyways? Because we used to assign lock names based on a descripter, like "kubeConfig", and we wanted multiple users to be able to run minikube simultaneously. Now that we only generate locks on a path name, this is no longer necessary, as we actually do want to lock a file if multiple users try to update it.
Fixes #6058