ttrpc: use os.Getuid/os.Getgid directly #13
Merged
Conversation
This file contains 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
Because of issues with glibc, using the `os/user` package can cause when calling `user.Current()`. Neither the Go maintainers or glibc developers could be bothered to fix it, so we have to work around it by calling the uid and gid functions directly. This is probably better because we don't actually use much of the data provided in the `user.User` struct. This required some refactoring to have better control over when the uid and gid are resolved. Rather than checking the current user on every connection, we now resolve it once at initialization. To test that this provided an improvement in performance, a benchmark was added. Unfortunately, this exposed a regression in the performance of unix sockets in Go when `(*UnixConn).File` is called. The underlying culprit of this performance regression is still at large. The following open issues describe the underlying problem in more detail: golang/go#13470 https://sourceware.org/bugzilla/show_bug.cgi?id=19341 In better news, I now have an entire herd of shaved yaks. Signed-off-by: Stephen J Day <stephen.day@docker.com>
jonjohnsonjr
referenced
this pull request
in moby/moby
Sep 6, 2019
Signed-off-by: Ahmet Alp Balkan <ahmetb@microsoft.com>
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.
Because of issues with glibc, using the
os/userpackage can cause whencalling
user.Current(). Neither the Go maintainers or glibc developerscould be bothered to fix it, so we have to work around it by calling the
uid and gid functions directly. This is probably better because we don't
actually use much of the data provided in the
user.Userstruct.This required some refactoring to have better control over when the uid
and gid are resolved. Rather than checking the current user on every
connection, we now resolve it once at initialization. To test that this
provided an improvement in performance, a benchmark was added.
Unfortunately, this exposed a regression in the performance of unix
sockets in Go when
(*UnixConn).Fileis called. The underlying culpritof this performance regression is still at large.
The following open issues describe the underlying problem in more
detail:
golang/go#13470
https://sourceware.org/bugzilla/show_bug.cgi?id=19341
In better news, I now have an entire herd of shaved yaks.
Signed-off-by: Stephen J Day stephen.day@docker.com