You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/04/02 23:16:00 UTC

[kudu-CR] KUDU-2395 (part 1): cache logged-in user

Hello Mike Percy,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/9895

to review the following change.


Change subject: KUDU-2395 (part 1): cache logged-in user
......................................................................

KUDU-2395 (part 1): cache logged-in user

The name of the current logged-in user is not expected to change over
time once the process is running. If it did change this could cause
strange effects with ACL enforcement, for one, or result in outbound RPC
connections switching credentials during the process lifetime.

Additionally, as pointed at in KUDU-2395, the lookup of the user name
from the UID sometimes needs to go to external services which may be
slow and incur lock contention.

This patch switches to computing the local username only once and
reusing the result on all subsequent lookups.

Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
---
M src/kudu/util/user.cc
1 file changed, 23 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/9895/1
-- 
To view, visit http://gerrit.cloudera.org:8080/9895
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
Gerrit-Change-Number: 9895
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-2395 (part 1): cache logged-in user

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9895 )

Change subject: KUDU-2395 (part 1): cache logged-in user
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/9895
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
Gerrit-Change-Number: 9895
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 03 Apr 2018 03:45:55 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2395 (part 1): cache logged-in user

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9895 )

Change subject: KUDU-2395 (part 1): cache logged-in user
......................................................................

KUDU-2395 (part 1): cache logged-in user

The name of the current logged-in user is not expected to change over
time once the process is running. If it did change this could cause
strange effects with ACL enforcement, for one, or result in outbound RPC
connections switching credentials during the process lifetime.

Additionally, as pointed at in KUDU-2395, the lookup of the user name
from the UID sometimes needs to go to external services which may be
slow and incur lock contention.

This patch switches to computing the local username only once and
reusing the result on all subsequent lookups.

Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
Reviewed-on: http://gerrit.cloudera.org:8080/9895
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/util/user.cc
1 file changed, 23 insertions(+), 2 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Todd Lipcon: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/9895
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
Gerrit-Change-Number: 9895
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-2395 (part 1): cache logged-in user

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9895 )

Change subject: KUDU-2395 (part 1): cache logged-in user
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/9895
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
Gerrit-Change-Number: 9895
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 04 Apr 2018 00:21:25 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2395 (part 1): cache logged-in user

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9895 )

Change subject: KUDU-2395 (part 1): cache logged-in user
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9895/1/src/kudu/util/user.cc
File src/kudu/util/user.cc:

http://gerrit.cloudera.org:8080/#/c/9895/1/src/kudu/util/user.cc@79
PS1, Line 79: 
> nit: extra semicolon
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/9895
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
Gerrit-Change-Number: 9895
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 03 Apr 2018 23:41:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2395 (part 1): cache logged-in user

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9895 )

Change subject: KUDU-2395 (part 1): cache logged-in user
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9895/1/src/kudu/util/user.cc
File src/kudu/util/user.cc:

http://gerrit.cloudera.org:8080/#/c/9895/1/src/kudu/util/user.cc@79
PS1, Line 79: ;
nit: extra semicolon



-- 
To view, visit http://gerrit.cloudera.org:8080/9895
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
Gerrit-Change-Number: 9895
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 03 Apr 2018 19:51:18 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2395 (part 1): cache logged-in user

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/9895

to look at the new patch set (#2).

Change subject: KUDU-2395 (part 1): cache logged-in user
......................................................................

KUDU-2395 (part 1): cache logged-in user

The name of the current logged-in user is not expected to change over
time once the process is running. If it did change this could cause
strange effects with ACL enforcement, for one, or result in outbound RPC
connections switching credentials during the process lifetime.

Additionally, as pointed at in KUDU-2395, the lookup of the user name
from the UID sometimes needs to go to external services which may be
slow and incur lock contention.

This patch switches to computing the local username only once and
reusing the result on all subsequent lookups.

Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
---
M src/kudu/util/user.cc
1 file changed, 23 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/9895/2
-- 
To view, visit http://gerrit.cloudera.org:8080/9895
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4500d89dd91e8eed6e4ae0661b886ab3c105130
Gerrit-Change-Number: 9895
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>