You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2019/04/30 23:58:58 UTC

[kudu-CR] [authz] add scrubbing task for the privileges cache

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13205


Change subject: [authz] add scrubbing task for the privileges cache
......................................................................

[authz] add scrubbing task for the privileges cache

This changelist enables the periodic scrubbing thread to invalidate
expired entries from the SentryPrivilegesFetcher's cache.  The
period of the scrubbing thread is a half of an entry's TTL.

Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
---
M src/kudu/master/sentry_privileges_fetcher.cc
1 file changed, 5 insertions(+), 3 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13205/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13205/1//COMMIT_MSG@10
PS1, Line 10: The
            : period of the scrubbing thread is a half of an entry's TTL.
> How did you arrive at this value?
Updated -- I decided to go with 20 seconds by default.  The idea is that the TTL for an entry in privileges cache might be relatively long, so it doesn't make sense to make it dependent on that parameter.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 01 May 2019 08:01:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 03 May 2019 00:34:24 +0000
Gerrit-HasComments: No

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13205/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13205/1//COMMIT_MSG@10
PS1, Line 10: The
            : period of the scrubbing thread is a half of an entry's TTL.
How did you arrive at this value?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 01 May 2019 00:04:00 +0000
Gerrit-HasComments: Yes

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 5: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13205/5/src/kudu/master/sentry_privileges_fetcher.cc
File src/kudu/master/sentry_privileges_fetcher.cc:

http://gerrit.cloudera.org:8080/#/c/13205/5/src/kudu/master/sentry_privileges_fetcher.cc@137
PS5, Line 137: TAG_FLAG(sentry_privileges_cache_scrubbing_period_sec, experimental);
What do you think about making these non-experimental?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 May 2019 20:32:23 +0000
Gerrit-HasComments: Yes

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 May 2019 18:42:43 +0000
Gerrit-HasComments: No

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13205/4/src/kudu/master/sentry_privileges_fetcher.cc
File src/kudu/master/sentry_privileges_fetcher.cc:

http://gerrit.cloudera.org:8080/#/c/13205/4/src/kudu/master/sentry_privileges_fetcher.cc@131
PS4, Line 131: DEFINE_uint32(sentry_privileges_cache_scrubbing_period_sec, 20,
The implication here that maybe should be noted (though maybe not) is that every 20 seconds, we'll lock the cache to do the scrubbing. In the happy case where there isn't much to scrub, we'll short-circuit early because we'll see a valid entry in the cache. In the unhappy case where there is a lot to scrub, the locking is warranted because there's a decent amount of work being done (i.e. many entries are being deleted).

Probably not worth adding all that to the description, but just validating my understanding of the scrubbing.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 01 May 2019 20:17:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] [authz] add scrubbing task for the privileges cache

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/13205 )

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/13205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>

[kudu-CR] [authz] add scrubbing task for the privileges cache

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, 

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

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

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................

[authz] add scrubbing task for the privileges cache

This changelist enables the periodic scrubbing thread to invalidate
expired entries from the SentryPrivilegesFetcher's cache.  By default,
the period of the cache's scrubbing task is 20 seconds.

Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
---
M src/kudu/master/sentry_privileges_fetcher.cc
1 file changed, 30 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/13205/5
-- 
To view, visit http://gerrit.cloudera.org:8080/13205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [authz] add scrubbing task for the privileges cache

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Andrew Wong, Adar Dembo, Hao Hao, 

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

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

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................

[authz] add scrubbing task for the privileges cache

This changelist enables the periodic scrubbing thread to invalidate
expired entries from the SentryPrivilegesFetcher's cache.  By default,
the period of the cache's scrubbing task is 20 seconds.

Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
---
M src/kudu/master/sentry_privileges_fetcher.cc
1 file changed, 28 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/13205/6
-- 
To view, visit http://gerrit.cloudera.org:8080/13205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................

[authz] add scrubbing task for the privileges cache

This changelist enables the periodic scrubbing thread to invalidate
expired entries from the SentryPrivilegesFetcher's cache.  By default,
the period of the cache's scrubbing task is 20 seconds.

Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Reviewed-on: http://gerrit.cloudera.org:8080/13205
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/master/sentry_privileges_fetcher.cc
1 file changed, 28 insertions(+), 3 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [authz] add scrubbing task for the privileges cache

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, 

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

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

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................

[authz] add scrubbing task for the privileges cache

This changelist enables the periodic scrubbing thread to invalidate
expired entries from the SentryPrivilegesFetcher's cache.  By default,
the period of the cache's scrubbing task is 20 seconds.

Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
---
M src/kudu/master/sentry_privileges_fetcher.cc
1 file changed, 20 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/13205/4
-- 
To view, visit http://gerrit.cloudera.org:8080/13205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13205/4/src/kudu/master/sentry_privileges_fetcher.cc
File src/kudu/master/sentry_privileges_fetcher.cc:

http://gerrit.cloudera.org:8080/#/c/13205/4/src/kudu/master/sentry_privileges_fetcher.cc@131
PS4, Line 131: DEFINE_uint32(sentry_privileges_cache_scrubbing_period_sec, 20,
> The implication here that maybe should be noted (though maybe not) is that 
Yep, that's right.  And the locking issue has been addressed now.

I added a short summary of your comment into the description of the newly introduced parameter.


http://gerrit.cloudera.org:8080/#/c/13205/4/src/kudu/master/sentry_privileges_fetcher.cc@133
PS4, Line 133: A value of 0 means "
             :               "'do not spawn the scrubbing thread', so expired entries are "
             :               "evicted only when the cache is at capacity while trying "
             :               "to accommodate new entries."
> Nit: more terse as "A value of 0 means expired entries are only evicted whe
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 May 2019 01:37:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 5: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 May 2019 03:27:06 +0000
Gerrit-HasComments: No

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13205/4/src/kudu/master/sentry_privileges_fetcher.cc
File src/kudu/master/sentry_privileges_fetcher.cc:

http://gerrit.cloudera.org:8080/#/c/13205/4/src/kudu/master/sentry_privileges_fetcher.cc@133
PS4, Line 133: A value of 0 means "
             :               "'do not spawn the scrubbing thread', so expired entries are "
             :               "evicted only when the cache is at capacity while trying "
             :               "to accommodate new entries."
Nit: more terse as "A value of 0 means expired entries are only evicted when inserting new entries into a full cache."



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 01 May 2019 16:22:42 +0000
Gerrit-HasComments: Yes

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 5: Verified+1

unrelated flakes in:
  * token-test
  * org.apache.kudu.client.TestSecurity
  * org.apache.kudu.spark.tools.DistributedDataGeneratorTest
  * org.apache.kudu.client.TestTimeouts


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 May 2019 04:24:19 +0000
Gerrit-HasComments: No

[kudu-CR] [authz] add scrubbing task for the privileges cache

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13205/5/src/kudu/master/sentry_privileges_fetcher.cc
File src/kudu/master/sentry_privileges_fetcher.cc:

http://gerrit.cloudera.org:8080/#/c/13205/5/src/kudu/master/sentry_privileges_fetcher.cc@137
PS5, Line 137: TAG_FLAG(sentry_privileges_cache_scrubbing_period_sec, experimental);
> What do you think about making these non-experimental?
I think if we don't think to remove this configuration option any soon, we can drop the 'experimental' tag right away.

We can also have some controls for that via kudu CLI, but I think having it as a flag is the best way to go.  I'll remove the 'experimental' tag from both of the parameters.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 May 2019 23:52:56 +0000
Gerrit-HasComments: Yes

[kudu-CR] [authz] add scrubbing task for the privileges cache

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, 

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

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

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

Change subject: [authz] add scrubbing task for the privileges cache
......................................................................

[authz] add scrubbing task for the privileges cache

This changelist enables the periodic scrubbing thread to invalidate
expired entries from the SentryPrivilegesFetcher's cache.  The
period of the scrubbing thread is a half of an entry's TTL.

Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
---
M src/kudu/master/sentry_privileges_fetcher.cc
1 file changed, 20 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/13205/3
-- 
To view, visit http://gerrit.cloudera.org:8080/13205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88510fae48cf683fbfad8a11c2941a23b2af9f8b
Gerrit-Change-Number: 13205
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)