You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2020/02/18 00:39:28 UTC
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15235
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Fix the flakiness of Ranger FE tests in AuthorizationStmtTest which is
caused by a row filter policy not being cleanly deleted. There is a bug
in Ranger that policies being deleted in Ranger Admin server but still
exist in Ranger plugins when there are concurrent create policy and get
policy requests (RANGER-2727). It's more possible to hit the bug if we
have more ranger plugins running, since each plugin instance will poll
policies in each 30s regularly.
Impalad and Catalogd servers only initialize one ImpalaRangerPlugin
instance. However, AuthorizationStmtTest has embedded Frontend and
CatalogServiceCatalog objects. It will initialize two ranger plugin
instances totally. What's worse, the JUnit testing framework make a new
object for each test method run. Currently there are 29 test methods in
AuthorizationStmtTest, which means 29 AuthorizationStmtTest objects will
be created. So we finally have 58 ranger plugin instances running, which
make RANGER-2727 easy to happens.
The failure can be reproduced by adding the following new test and run
it with all existing tests:
@Test
public void testRangerPolicyRepeatedly() throws Exception {
if (authzProvider_ == AuthorizationProvider.SENTRY) return;
for (int i = 0; i < 100; ++i) {
testRowFilterEnabled();
testColumnMaskEnabled();
}
}
We only explicitly create policies for column masking and row filtering
(other tests are using grant/revoke requests). This test increases the
number of create policy requests, so increases the possibility with
concurrent get policies requests polling from other ranger plugin
instances created by previous tests.
The fix is to make ImpalaRangerPlugin a singleton class so we will have
only one ranger plugin instance, which dramatically reduces the
possibility of hitting RANGER-2727. The thorough fix is bumping CDP
version after RANGER-2727 is resolved. Codes added in the previous patch
(c1244c2f04e629cc07b0830a597c70317be92768) are removed.
Tests:
- Ran AuthorizationStmtTest with the above new test.
Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
5 files changed, 38 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/15235/1
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 13:08:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Feb 2020 06:38:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5245/ : 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/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 01:25:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java:
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java@30
PS2, Line 30: erImpalaPlugin ext
> As far as I know (I am not a java expert) INSTANCE should be voletile for d
Yes! We need volatile.
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java@39
PS2, Line 39: public static RangerImpalaPlugin getInstance(String serviceType, String appId) {
: if (INSTANCE == null) {
: synchronized(RangerImpalaPlugin.class) {
: if (INSTANCE == null) {
> nit: This could be skipped if there are two getInstance() calls in parallel
Nice find!
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 11:02:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 05:47:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5351/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 01:28:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5250/ : 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/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 11:43:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 12:52:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Fang-Yu Rao, Vihang Karajgaonkar, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15235
to look at the new patch set (#4).
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Fix the flakiness of Ranger FE tests in AuthorizationStmtTest which is
caused by a row filter policy not being cleanly deleted. There is a bug
in Ranger that policies being deleted in Ranger Admin server but still
exist in Ranger plugins when there are concurrent create policy and get
policy requests (RANGER-2727). It's more possible to hit the bug if we
have more ranger plugins running, since each plugin instance will poll
policies in each 30s regularly.
Impalad and Catalogd servers only initialize one ImpalaRangerPlugin
instance. However, AuthorizationStmtTest has embedded Frontend and
CatalogServiceCatalog objects. It will initialize two ranger plugin
instances totally. What's worse, the JUnit testing framework makes a new
object for each test method run. Currently there are 29 test methods in
AuthorizationStmtTest, which means 29 AuthorizationStmtTest objects will
be created. So we finally have 58 ranger plugin instances running, which
makes RANGER-2727 easy to happen.
The failure can be reproduced by adding the following new test and run
it with all existing tests:
@Test
public void testRangerPolicyRepeatedly() throws Exception {
if (authzProvider_ == AuthorizationProvider.SENTRY) return;
for (int i = 0; i < 100; ++i) {
testRowFilterEnabled();
testColumnMaskEnabled();
}
}
We only explicitly create policies for column masking and row filtering
(other tests are using grant/revoke requests). This test increases the
number of CreatePolicy requests, so increases the possibility of
CreatePolicy requests running concurrently with GetPolicies requests
polling from other ranger plugin instances created by previous tests.
The fix is to make ImpalaRangerPlugin a singleton class so we will have
only one ranger plugin instance, which dramatically reduces the
possibility of hitting RANGER-2727. The thorough fix is bumping CDP
version after RANGER-2727 is resolved. Codes added in the previous patch
(c1244c2f04e629cc07b0830a597c70317be92768) are removed.
Tests:
- Ran AuthorizationStmtTest with the above new test.
Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
5 files changed, 39 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/15235/4
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Anurag Mantripragada, Fang-Yu Rao, Vihang Karajgaonkar, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15235
to look at the new patch set (#3).
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Fix the flakiness of Ranger FE tests in AuthorizationStmtTest which is
caused by a row filter policy not being cleanly deleted. There is a bug
in Ranger that policies being deleted in Ranger Admin server but still
exist in Ranger plugins when there are concurrent create policy and get
policy requests (RANGER-2727). It's more possible to hit the bug if we
have more ranger plugins running, since each plugin instance will poll
policies in each 30s regularly.
Impalad and Catalogd servers only initialize one ImpalaRangerPlugin
instance. However, AuthorizationStmtTest has embedded Frontend and
CatalogServiceCatalog objects. It will initialize two ranger plugin
instances totally. What's worse, the JUnit testing framework make a new
object for each test method run. Currently there are 29 test methods in
AuthorizationStmtTest, which means 29 AuthorizationStmtTest objects will
be created. So we finally have 58 ranger plugin instances running, which
makes RANGER-2727 easy to happens.
The failure can be reproduced by adding the following new test and run
it with all existing tests:
@Test
public void testRangerPolicyRepeatedly() throws Exception {
if (authzProvider_ == AuthorizationProvider.SENTRY) return;
for (int i = 0; i < 100; ++i) {
testRowFilterEnabled();
testColumnMaskEnabled();
}
}
We only explicitly create policies for column masking and row filtering
(other tests are using grant/revoke requests). This test increases the
number of create policy requests, so increases the possibility with
concurrent get policies requests polling from other ranger plugin
instances created by previous tests.
The fix is to make ImpalaRangerPlugin a singleton class so we will have
only one ranger plugin instance, which dramatically reduces the
possibility of hitting RANGER-2727. The thorough fix is bumping CDP
version after RANGER-2727 is resolved. Codes added in the previous patch
(c1244c2f04e629cc07b0830a597c70317be92768) are removed.
Tests:
- Ran AuthorizationStmtTest with the above new test.
Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
5 files changed, 39 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/15235/3
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java:
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java@30
PS2, Line 30: RangerImpalaPlugin
As far as I know (I am not a java expert) INSTANCE should be voletile for double-checked locking to work for singletons.
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java@39
PS2, Line 39: Preconditions.checkState(SERVICE_TYPE == null || SERVICE_TYPE.equals(serviceType),
: String.format("%s != %s", SERVICE_TYPE, serviceType));
: Preconditions.checkState(APP_ID == null || APP_ID.equals(appId),
: String.format("%s != %s", APP_ID, appId));
nit: This could be skipped if there are two getInstance() calls in parallel with different configs and INSTANCE is still null. Moving the tests to the end seem more logical to me.
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 10:08:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5244/ : 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/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 01:21:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Fix the flakiness of Ranger FE tests in AuthorizationStmtTest which is
caused by a row filter policy not being cleanly deleted. There is a bug
in Ranger that policies being deleted in Ranger Admin server but still
exist in Ranger plugins when there are concurrent create policy and get
policy requests (RANGER-2727). It's more possible to hit the bug if we
have more ranger plugins running, since each plugin instance will poll
policies in each 30s regularly.
Impalad and Catalogd servers only initialize one ImpalaRangerPlugin
instance. However, AuthorizationStmtTest has embedded Frontend and
CatalogServiceCatalog objects. It will initialize two ranger plugin
instances totally. What's worse, the JUnit testing framework makes a new
object for each test method run. Currently there are 29 test methods in
AuthorizationStmtTest, which means 29 AuthorizationStmtTest objects will
be created. So we finally have 58 ranger plugin instances running, which
makes RANGER-2727 easy to happen.
The failure can be reproduced by adding the following new test and run
it with all existing tests:
@Test
public void testRangerPolicyRepeatedly() throws Exception {
if (authzProvider_ == AuthorizationProvider.SENTRY) return;
for (int i = 0; i < 100; ++i) {
testRowFilterEnabled();
testColumnMaskEnabled();
}
}
We only explicitly create policies for column masking and row filtering
(other tests are using grant/revoke requests). This test increases the
number of CreatePolicy requests, so increases the possibility of
CreatePolicy requests running concurrently with GetPolicies requests
polling from other ranger plugin instances created by previous tests.
The fix is to make ImpalaRangerPlugin a singleton class so we will have
only one ranger plugin instance, which dramatically reduces the
possibility of hitting RANGER-2727. The thorough fix is bumping CDP
version after RANGER-2727 is resolved. Codes added in the previous patch
(c1244c2f04e629cc07b0830a597c70317be92768) are removed.
Tests:
- Ran AuthorizationStmtTest with the above new test.
Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Reviewed-on: http://gerrit.cloudera.org:8080/15235
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
5 files changed, 39 insertions(+), 37 deletions(-)
Approvals:
Csaba Ringhofer: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5359/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Feb 2020 02:02:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15235
to look at the new patch set (#2).
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
......................................................................
IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Fix the flakiness of Ranger FE tests in AuthorizationStmtTest which is
caused by a row filter policy not being cleanly deleted. There is a bug
in Ranger that policies being deleted in Ranger Admin server but still
exist in Ranger plugins when there are concurrent create policy and get
policy requests (RANGER-2727). It's more possible to hit the bug if we
have more ranger plugins running, since each plugin instance will poll
policies in each 30s regularly.
Impalad and Catalogd servers only initialize one ImpalaRangerPlugin
instance. However, AuthorizationStmtTest has embedded Frontend and
CatalogServiceCatalog objects. It will initialize two ranger plugin
instances totally. What's worse, the JUnit testing framework make a new
object for each test method run. Currently there are 29 test methods in
AuthorizationStmtTest, which means 29 AuthorizationStmtTest objects will
be created. So we finally have 58 ranger plugin instances running, which
makes RANGER-2727 easy to happens.
The failure can be reproduced by adding the following new test and run
it with all existing tests:
@Test
public void testRangerPolicyRepeatedly() throws Exception {
if (authzProvider_ == AuthorizationProvider.SENTRY) return;
for (int i = 0; i < 100; ++i) {
testRowFilterEnabled();
testColumnMaskEnabled();
}
}
We only explicitly create policies for column masking and row filtering
(other tests are using grant/revoke requests). This test increases the
number of create policy requests, so increases the possibility with
concurrent get policies requests polling from other ranger plugin
instances created by previous tests.
The fix is to make ImpalaRangerPlugin a singleton class so we will have
only one ranger plugin instance, which dramatically reduces the
possibility of hitting RANGER-2727. The thorough fix is bumping CDP
version after RANGER-2727 is resolved. Codes added in the previous patch
(c1244c2f04e629cc07b0830a597c70317be92768) are removed.
Tests:
- Ran AuthorizationStmtTest with the above new test.
Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
5 files changed, 38 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/15235/2
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>