You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2019/06/14 21:22:07 UTC
[kudu-CR] [sentry] add require db privileges flag for ListTables
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13657
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
[sentry] add require_db_privileges flag for ListTables
This patch adds a sentry_require_db_privileges_for_list_tables flag to
allow enforcing database level privileges for ListTables. The motivation
is to reduce time spend on authorization when there are a small amount
of databases but many tables in these. However, note that when the flag
is set to true, users with no database level privileges on a database
will not be able to see any tables within it.
Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
2 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/13657/1
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
[sentry] add require_db_privileges flag for ListTables
This patch adds a sentry_require_db_privileges_for_list_tables flag to
allow enforcing database level privileges for ListTables. Without this
flag, there will be a number of requests to Sentry related to the number
of tables in Kudu. With it, that number is limited to the number of
databases in Kudu. However, note that when the flag is set to true,
users with no database-level privileges on a database will not be able
to see any tables within it.
Using ListTablesBenchmark without the flag:
$ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=false
sentry_authz_provider-test.cc:421] Time spent Listing tables: real 122.567s user 0.339s sys 0.016s
With the flag:
$ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=true
sentry_authz_provider-test.cc:421] Time spent Listing tables: real 1.869s user 0.011s sys 0.000s
Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Reviewed-on: http://gerrit.cloudera.org:8080/13657
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
2 files changed, 18 insertions(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Hao Hao: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13657
to look at the new patch set (#2).
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
[sentry] add require_db_privileges flag for ListTables
This patch adds a sentry_require_db_privileges_for_list_tables flag to
allow enforcing database level privileges for ListTables. Without this
flag, there will be a number of requests to Sentry related to the number
of tables in Kudu. With it, that number is limited to the number of
databases in Kudu. However, note that when the flag is set to true,
users with no database-level privileges on a database will not be able
to see any tables within it.
Using ListTablesBenchmark without the flag:
$ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=false
sentry_authz_provider-test.cc:421] Time spent Listing tables: real 122.567s user 0.339s sys 0.016s
With the flag:
$ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=true --sentry_require_db_privileges_for_list_tables=true
sentry_authz_provider-test.cc:421] Time spent Listing tables: real 1.838s user 0.009s sys 0.001s
Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
2 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/13657/2
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13657/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13657/2//COMMIT_MSG@23
PS2, Line 23: --has-db-privileges=true
This isn't an apples-to-apples comparison, right? If the user has all db privileges, it doesn't matter whether --sentry_require_db_privileges_for_list_tables is true or not.
Should probably set this to be the same value in both tests. Or replace --has-db-privileges with a --num-db-privileges configuration for the benchmark, where if d < FLAGS_num_db_privileges, grant the privilege on the db and if not, then grant the dummy privilege in ListTablesBenchmark.
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 15 Jun 2019 00:30:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13657/3/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/13657/3/src/kudu/master/sentry_authz_provider.cc@47
PS3, Line 47: TAG_FLAG(sentry_require_db_privileges_for_list_tables, evolving);
nit: 'evolving' is the default for a flag, so you could drop this
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 15 Jun 2019 02:30:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 16 Jun 2019 19:38:52 +0000
Gerrit-HasComments: No
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13657
to look at the new patch set (#3).
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
[sentry] add require_db_privileges flag for ListTables
This patch adds a sentry_require_db_privileges_for_list_tables flag to
allow enforcing database level privileges for ListTables. Without this
flag, there will be a number of requests to Sentry related to the number
of tables in Kudu. With it, that number is limited to the number of
databases in Kudu. However, note that when the flag is set to true,
users with no database-level privileges on a database will not be able
to see any tables within it.
Using ListTablesBenchmark without the flag:
$ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=false
sentry_authz_provider-test.cc:421] Time spent Listing tables: real 122.567s user 0.339s sys 0.016s
With the flag:
$ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=true
sentry_authz_provider-test.cc:421] Time spent Listing tables: real 1.869s user 0.011s sys 0.000s
Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
2 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/13657/3
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13657/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13657/1//COMMIT_MSG@10
PS1, Line 10: Without this
: flag, there will be a number of requests to Sentry related to the number
: of tables in Kudu. With it, that numbe
> It may be obvious if you read between the lines, but it's probably worth be
Done
http://gerrit.cloudera.org:8080/#/c/13657/1/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/13657/1/src/kudu/master/sentry_authz_provider.cc@41
PS1, Line 41:
: DEFINE_bool(sentry_require_db_privileges_for_list_tables, false,
: "Whether Kudu will require database-level privileges to authorize "
: "ListTables requests. When set to false, table-level privileges are "
: "required f
> maybe: "Whether Kudu will require database-level privileges to authorize Li
Done
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 15 Jun 2019 00:05:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13657/3/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/13657/3/src/kudu/master/sentry_authz_provider.cc@47
PS3, Line 47: TAG_FLAG(sentry_require_db_privileges_for_list_tables, evolving);
> nit: 'evolving' is the default for a flag, so you could drop this
Done
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 16 Jun 2019 18:16:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13657/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13657/2//COMMIT_MSG@23
PS2, Line 23: --has-db-privileges=false
> This isn't an apples-to-apples comparison, right? If the user has all db pr
Done
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 15 Jun 2019 00:53:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13657
to look at the new patch set (#4).
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
[sentry] add require_db_privileges flag for ListTables
This patch adds a sentry_require_db_privileges_for_list_tables flag to
allow enforcing database level privileges for ListTables. Without this
flag, there will be a number of requests to Sentry related to the number
of tables in Kudu. With it, that number is limited to the number of
databases in Kudu. However, note that when the flag is set to true,
users with no database-level privileges on a database will not be able
to see any tables within it.
Using ListTablesBenchmark without the flag:
$ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=false
sentry_authz_provider-test.cc:421] Time spent Listing tables: real 122.567s user 0.339s sys 0.016s
With the flag:
$ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=true
sentry_authz_provider-test.cc:421] Time spent Listing tables: real 1.869s user 0.011s sys 0.000s
Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
2 files changed, 18 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/13657/4
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 15 Jun 2019 01:19:45 +0000
Gerrit-HasComments: No
[kudu-CR] [sentry] add require db privileges flag for ListTables
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13657 )
Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13657/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13657/1//COMMIT_MSG@10
PS1, Line 10: The motivation
: is to reduce time spend on authorization when there are a small amount
: of databases but many tables in these.
It may be obvious if you read between the lines, but it's probably worth being explicit about the expected performance behavior. Something like:
"Without this flag, there will be a number of requests to Sentry related to the number of tables in Kudu. With it, that number is limited to the number of databases in Kudu."
or even a concrete example:
"If there are 2000 tables stored in Kudu, equally stored within 10 databases, and a user has privileges on one databases but not the others, with the flag turned on, there will be 10 requests to Sentry instead of 1801."
Might also be worth running a couple benchmarks demonstrating this.
http://gerrit.cloudera.org:8080/#/c/13657/1/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/13657/1/src/kudu/master/sentry_authz_provider.cc@41
PS1, Line 41: "When use sentry for fine-grained authorization, ListTables requires "
: "any privileges on database level when set to true. Note that users "
: "with no database level privileges on a database will not be able to "
: "see any tables within it. When set to false table level privileges are "
: "enforced."
maybe: "Whether Kudu will require database-level privileges to authorize ListTables requests. When set to false, table-level privileges are required for each table."
--
To view, visit http://gerrit.cloudera.org:8080/13657
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13657
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 14 Jun 2019 22:42:04 +0000
Gerrit-HasComments: Yes