[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 10: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 18 Aug 2018 07:31:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..

IMPALA-7343: Update SentryProxy to use Sentry bulk API

Prior to this patch, every Sentry policy update required a Thrift call
to Sentry to get the role privileges per role. This is inefficient.
This patch updates the way Impala update its Sentry policy by using the
new Sentry APIs for getting all roles/users and their associated
privileges in a single Thrift call to Sentry. This patch also updates
SentryProxy to get user privileges.

Before:
- Refreshing 100 roles: 493ms
- Refreshing 1000 roles: 4668ms
- Refreshing 1 roles: 45636ms

After:
- Refreshing 100 roles: 114ms
- Refreshing 1000 roles: 1021ms
- Refreshing 1 roles: 10076ms

Testing:
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Reviewed-on: http://gerrit.cloudera.org:8080/11250
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M bin/impala-config.sh
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
4 files changed, 195 insertions(+), 73 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3040/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 18 Aug 2018 04:14:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 18 Aug 2018 04:14:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 9:

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3032/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 18 Aug 2018 02:12:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3032/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 23:03:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 9:

> Patch Set 9: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3031/

A lot of query test failures which I don't think it's related to this change:

query_test.test_nested_types.TestNestedTypes.test_subplan[exec_option: 
{'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 
'disable_codegen': False, 'abort_on_error': 1, 'debug_action': None, 
'exec_single_node_rows_threshold': 0} | table_format: parquet/none]
query_test.test_scanners.TestParquet.test_multi_compression_types[exec_option: 
{'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 
'disable_codegen': True, 'abort_on_error': 1, 'debug_action': None, 
'exec_single_node_rows_threshold': 0} | table_format: parquet/none]
query_test.test_scratch_limit.TestScratchLimit.test_with_zero_scratch_limit_no_memory_limit[exec_option:
 {'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 5000, 
'disable_codegen': False, 'abort_on_error': 1, 'debug_action': None, 
'exec_single_node_rows_threshold': 0} | table_format: text/none]
query_test.test_scanners_fuzz.TestScannersFuzzing.test_fuzz_uncompressed_parquet[exec_option:
 {'debug_action': '-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@1.0', 
'abort_on_error': False, 'mem_limit': '512m', 'num_nodes': 0} | table_format: 
parquet/none]
query_test.test_kudu.TestKuduOperations.test_kudu_partition_ddl[exec_option: 
{'kudu_read_mode': 'READ_AT_SNAPSHOT', 'batch_size': 0, 'num_nodes': 0, 
'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 
'abort_on_error': 1, 'debug_action': None, 'exec_single_node_rows_threshold': 
0} | table_format: text/none]
query_test.test_tpcds_queries.TestTpcdsQuery.test_tpcds_q40[exec_option: 
{'decimal_v2': 0, 'batch_size': 0, 'num_nodes': 0, 
'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 
'abort_on_error': 1, 'debug_action': None, 'exec_single_node_rows_threshold': 
0} | table_format: parquet/none]
query_test.test_tpcds_queries.TestTpcdsQuery.test_tpcds_q18a[exec_option: 
{'decimal_v2': 0, 'batch_size': 0, 'num_nodes': 0, 
'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 
'abort_on_error': 1, 'debug_action': None, 'exec_single_node_rows_threshold': 
0} | table_format: parquet/none]
query_test.test_insert_parquet.TestInsertParquetVerifySize.test_insert_parquet_verify_size[compression_codec:
 gzip | exec_option: {'sync_ddl': 1, 'batch_size': 0, 'num_nodes': 0, 
'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 
'abort_on_error': 1, 'debug_action': None, 'exec_single_node_rows_threshold': 
0} | table_format: parquet/none]
query_test.test_scanners_fuzz.TestScannersFuzzing.test_fuzz_alltypes[exec_option:
 {'debug_action': None, 'abort_on_error': False, 'mem_limit': '512m', 
'num_nodes': 0} | table_format: avro/snap/block]
query_test.test_scanners.TestParquet.test_decimal_encodings[exec_option: 
{'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 
'disable_codegen': True, 'abort_on_error': 1, 'debug_action': None, 
'exec_single_node_rows_threshold': 0} | table_format: parquet/none]
Show all failed tests >>>


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 22:56:19 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 9: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3031/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 22:41:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/396/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 20:26:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/394/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 19:59:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3031/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 19:44:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 8: Code-Review+2

(1 comment)

Carry Vuk's +2.

http://gerrit.cloudera.org:8080/#/c/11250/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11250/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java@150
PS7, Line 150: tho
> nit: remove
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 19:44:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 19:44:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..

IMPALA-7343: Update SentryProxy to use Sentry bulk API

Prior to this patch, every Sentry policy update required a Thrift call
to Sentry to get the role privileges per role. This is inefficient.
This patch updates the way Impala update its Sentry policy by using the
new Sentry APIs for getting all roles/users and their associated
privileges in a single Thrift call to Sentry. This patch also updates
SentryProxy to get user privileges.

Before:
- Refreshing 100 roles: 493ms
- Refreshing 1000 roles: 4668ms
- Refreshing 1 roles: 45636ms

After:
- Refreshing 100 roles: 114ms
- Refreshing 1000 roles: 1021ms
- Refreshing 1 roles: 10076ms

Testing:
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
---
M bin/impala-config.sh
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
4 files changed, 195 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/11250/8
--
To view, visit http://gerrit.cloudera.org:8080/11250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 7: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11250/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11250/7/fe/src/main/java/org/apache/impala/util/SentryProxy.java@150
PS7, Line 150: the
nit: remove



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 19:38:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..

IMPALA-7343: Update SentryProxy to use Sentry bulk API

Prior to this patch, every Sentry policy update required a Thrift call
to Sentry to get the role privileges per role. This is inefficient.
This patch updates the way Impala update its Sentry policy by using the
new Sentry APIs for getting all roles/users and their associated
privileges in a single Thrift call to Sentry. This patch also updates
SentryProxy to get user privileges.

Before:
- Refreshing 100 roles: 493ms
- Refreshing 1000 roles: 4668ms
- Refreshing 1 roles: 45636ms

After:
- Refreshing 100 roles: 114ms
- Refreshing 1000 roles: 1021ms
- Refreshing 1 roles: 10076ms

Testing:
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
---
M bin/impala-config.sh
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
4 files changed, 195 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/11250/7
--
To view, visit http://gerrit.cloudera.org:8080/11250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
File fe/src/main/java/org/apache/impala/util/SentryPolicyService.java:

http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java@460
PS6, Line 460: throw new 
AuthorizationException(String.format(ACCESS_DENIED_ERROR_MSG,
 : requestingUser.getName(),
 : type == TPrincipalType.ROLE ?
 : "LIST_ALL_ROLES_PRIVILEGES" : 
"LIST_ALL_USERS_PRIVILEGES"));
 :   }
 :   throw new InternalException(String.format("Error making 
'%s' RPC to " +
 :
> nit: can you merge these even more? e.g, add
Done


http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryProxy.java@149
PS6, Line 149: Updates a
> pls be more specific with what you mean by "refresh". does it mean the loca
Done


http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryProxy.java@223
PS6, Line 223:
> similar clarification here.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 19:26:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 6: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryProxy.java@149
PS6, Line 149: Refreshes
pls be more specific with what you mean by "refresh". does it mean the local 
state will be updated to reflect adds, removals, and replacements that have 
been made to Sentry since the last check?


http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryProxy.java@223
PS6, Line 223: Refreshes
similar clarification here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 17:50:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/390/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 17:33:58 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 6: Code-Review+1

(1 comment)

Thanks for the refactors! I added a comment about removing a duplication, lgtm 
otherwise.

http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
File fe/src/main/java/org/apache/impala/util/SentryPolicyService.java:

http://gerrit.cloudera.org:8080/#/c/11250/6/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java@460
PS6, Line 460: if (type == TPrincipalType.ROLE) {
 :   throw new 
AuthorizationException(String.format(ACCESS_DENIED_ERROR_MSG,
 :   requestingUser.getName(), 
"LIST_ALL_ROLES_PRIVILEGES"));
 : } else {
 :   throw new 
AuthorizationException(String.format(ACCESS_DENIED_ERROR_MSG,
 :   requestingUser.getName(), 
"LIST_ALL_USERS_PRIVILEGES"));
 : }
nit: can you merge these even more? e.g, add
String function_name = type == TPrincipalType.ROLE ? 
"LIST_ALL_ROLES_PRIVILEGES" : "LIST_ALL_USERS_PRIVILEGES";

Something similar could be done for the other exception too.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 17:08:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG@10
PS4, Line 10: ine
> I guess that this "not" is not needed here.
Done


http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG@12
PS4, Line 12: rol
> double "all"
Done


http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
File fe/src/main/java/org/apache/impala/util/SentryPolicyService.java:

http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java@457
PS4, Line 457: s(requestingUser.getSh
> optional: I would prefer to merge this somehow with listAllRolesPrivileges.
Good idea. Done.


http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@243
PS4, Line 243: existingPrincipalPriv.setCatalogVersion(
 : catalog_.incrementAndGetCatalogVersion());
 :   }
 :   continue;
 : }
 : if (principal.getPrincipalType() == TPrincipalType.ROLE) 
{
 :   catalog_.addRolePrivilege(principal.getName(), 
thriftPriv);
 : } else {
 :   catalog_.addUserPrivilege(principal.getName(), 
thriftPriv);
 : }
 :   }
 :
 :   // Remove the privileges that no longer exist.
 :   for (String privilegeName: privilegesToRemove) {
 : TPrivilege privilege = new TPrivilege();
 : privilege.setPrivilege_name(privilegeName);
 : if (principal.getPrincipalType() == TPrincipalType.ROLE) 
{
 :   catalog_.removeRolePrivilege(principal.getName(), 
privilege);
 : } else {
 :   catalog_.removeUserPrivilege(principal.getName(), 
privilege);
 : }
 :   }
 : }
 :   }
 :
 :   /**
 :* Chec
> optional: this seems very similar to its pair in refreshRolePrivileges - it
Good idea. Done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 16:56:13 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..

IMPALA-7343: Update SentryProxy to use Sentry bulk API

Prior to this patch, every Sentry policy update required a Thrift call
to Sentry to get the role privileges per role. This is inefficient.
This patch updates the way Impala update its Sentry policy by using the
new Sentry APIs for getting all roles/users and their associated
privileges in a single Thrift call to Sentry. This patch also updates
SentryProxy to get user privileges.

Before:
- Refreshing 100 roles: 493ms
- Refreshing 1000 roles: 4668ms
- Refreshing 1 roles: 45636ms

After:
- Refreshing 100 roles: 114ms
- Refreshing 1000 roles: 1021ms
- Refreshing 1 roles: 10076ms

Testing:
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
---
M bin/impala-config.sh
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
4 files changed, 197 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/11250/6
--
To view, visit http://gerrit.cloudera.org:8080/11250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-17 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG@10
PS4, Line 10: not
I guess that this "not" is not needed here.


http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG@12
PS4, Line 12: all
double "all"


http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
File fe/src/main/java/org/apache/impala/util/SentryPolicyService.java:

http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java@457
PS4, Line 457: listAllUsersPrivileges
optional: I would prefer to merge this somehow with listAllRolesPrivileges. The 
way I see to achieve this is creating an enum like PrivilegeType {ROLE, USER} 
and use it to branch in the few lines where the two functions are different.


http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@243
PS4, Line 243: // Assume all privileges should be removed. Privileges 
that still exist are
 : // deleted from this set and we are left with the set of 
privileges that need
 : // to be removed.
 : Set privilegesToRemove = 
user.getPrivilegeNames();
 : for (TSentryPrivilege sentryPriv: 
userPrivilegesEntry.getValue()) {
 :   TPrivilege thriftPriv =
 :   
SentryPolicyService.sentryPrivilegeToTPrivilege(sentryPriv, user);
 :   
privilegesToRemove.remove(thriftPriv.getPrivilege_name().toLowerCase());
 :   PrincipalPrivilege existingPriv =
 :   user.getPrivilege(thriftPriv.getPrivilege_name());
 :   if (existingPriv != null &&
 :   existingPriv.getCreateTimeMs() == 
sentryPriv.getCreateTime()) {
 : if (resetVersions_) {
 :   existingPriv.setCatalogVersion(
 :   catalog_.incrementAndGetCatalogVersion());
 : }
 : continue;
 :   }
 :   catalog_.addUserPrivilege(user.getName(), thriftPriv);
 : }
 :
 : // Remove the privileges that no longer exist.
 : for (String privilegeName: privilegesToRemove) {
 :   TPrivilege privilege = new TPrivilege();
 :   privilege.setPrivilege_name(privilegeName);
 :   catalog_.removeUserPrivilege(user.getName(), 
privilege);
 : }
optional: this seems very similar to its pair in refreshRolePrivileges - it 
could be refactored to a separate function like RefreshPrivilegesInCatalog() 
which could decide between ROLE/USER logic based on a parameter.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 10:37:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-16 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/387/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 05:32:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-16 Thread Adam Holley (Code Review)
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 04:57:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-16 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11250/3/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11250/3/fe/src/main/java/org/apache/impala/util/SentryProxy.java@214
PS3, Line 214: Refreshes all users and their associated privileges
> Modify this comment to indicate it is only users that have privileges and n
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 04:55:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-16 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..

IMPALA-7343: Update SentryProxy to use Sentry bulk API

Prior to this patch, every Sentry policy update required a Thrift call
to Sentry to get the role privileges per role. This is not inefficient.
This patch updates the way Impala update its Sentry policy by using the
new Sentry APIs for getting all all roles/users and their associated
privileges in a single Thrift call to Sentry. This patch also updates
SentryProxy to get user privileges.

Before:
- Refreshing 100 roles: 493ms
- Refreshing 1000 roles: 4668ms
- Refreshing 1 roles: 45636ms

After:
- Refreshing 100 roles: 114ms
- Refreshing 1000 roles: 1021ms
- Refreshing 1 roles: 10076ms

Testing:
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
---
M bin/impala-config.sh
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
4 files changed, 199 insertions(+), 74 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/11250/4
--
To view, visit http://gerrit.cloudera.org:8080/11250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-16 Thread Adam Holley (Code Review)
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11250/3/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11250/3/fe/src/main/java/org/apache/impala/util/SentryProxy.java@214
PS3, Line 214: Refreshes all users and their associated privileges
Modify this comment to indicate it is only users that have privileges and not 
all users in any specific context.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 04:51:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-16 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11250 )

Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/380/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 17 Aug 2018 00:32:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7343: Update SentryProxy to use Sentry bulk API

2018-08-16 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11250


Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API
..

IMPALA-7343: Update SentryProxy to use Sentry bulk API

Prior to this patch, every Sentry policy update required a Thrift call
to Sentry to get the role privileges per role. This is not inefficient.
This patch updates the way Impala update its Sentry policy by using the
new Sentry APIs for getting all all roles/users and their associated
privileges in a single Thrift call to Sentry. This patch also updates
SentryProxy to get user privileges.

Before:
- Refreshing 100 roles: 493ms
- Refreshing 1000 roles: 4668ms
- Refreshing 1 roles: 45636ms

After:
- Refreshing 100 roles: 114ms
- Refreshing 1000 roles: 1021ms
- Refreshing 1 roles: 10076ms

Testing:
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
---
M bin/impala-config.sh
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
4 files changed, 197 insertions(+), 74 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/11250/3
--
To view, visit http://gerrit.cloudera.org:8080/11250
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304
Gerrit-Change-Number: 11250
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya