[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11279 )
Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
..
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
File
fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java:
http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java@22
PS1, Line 22: import org.apache.impala.common.AnalysisException;
> unused import
Done
http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1972
PS1, Line 1972: test.ok(onServer(true, TPrivilegeLevel.ALL))
: .ok(onServer(true, TPrivilegeLevel.OWNER))
: .ok(onDatabase(true, "functional",
TPrivilegeLevel.ALL))
: .ok(onDatabase(true, "functional",
TPrivilegeLevel.OWNER))
: .ok(onTable(true, "functional", "alltypes",
TPrivilegeLevel.ALL))
: .ok(onTable(true, "functional", "alltypes",
TPrivilegeLevel.OWNER))
> Should have failure tests for grant option false.
Done
http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1985
PS1, Line 1985: or(a
> nit: This indent is inconsistent with others. See previous test block.
Done
http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2188
PS1, Line 2188: TPrivilegeLevel.ALL, TPrivilegeLevel.OWNER,
TPrivilegeLevel.ALTER)))
: .error(alterError("functional.alltypes_view"),
onDatabase("functional", allExcept(
: TPrivilegeLevel.ALL, TPrivilegeLevel.OWNER,
TPrivilegeLevel.ALTER)))
: .error(alterError("functional.alltypes_view"),
onTable("functional",
: "alltypes_view", allExcept(TPrivilegeLevel.ALL,
TPrivilegeLevel.OWNER,
: TPrivilegeLevel.ALTER)));
> Should have error tests for grant option false;.
Done
http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2223
PS1, Line 2223: "alltypes_view", allExcept(TPrivilegeLevel.ALL,
TPrivilegeLevel.OWNER)));
: }
:
: // Database does not exist.
> Should have error tests for grant option false;.
Done
--
To view, visit http://gerrit.cloudera.org:8080/11279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3
Gerrit-Change-Number: 11279
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 21 Aug 2018 06:25:03 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Fredy Wijaya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements Prior to this patch, any user with ALTER privilege could alter the database/table ownership from one user/role to another user/role. This is undesirable because altering an object ownership means giving a full access to that object. This patch restricts the ALTER DATABASE/TABLE SET OWNER statements to require ALL/OWNER with GRANT OPTION when authorization is enabled. Testing: - Added FE authorization tests - Ran all FE tests - Ran core tests Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 --- M bin/impala-config.sh M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 15 files changed, 258 insertions(+), 77 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/11279/2 -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 Gerrit-PatchSet: 2 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-7422: Fix a race in QueryState::StartFInstances()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3052/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Aug 2018 06:21:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 4: GVO failed due to IMPALA-6776 / IMPALA-7061 -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Aug 2018 06:21:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 06:18:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11208 ) Change subject: IMPALA-7437. LRU caching of partitions in impalad .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 Gerrit-Change-Number: 11208 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 06:12:53 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11280 ) Change subject: WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3051/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I615f9e6bd167b36cd8d93da59426dd6813ae4984 Gerrit-Change-Number: 11280 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 06:11:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 06:11:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11208 ) Change subject: IMPALA-7437. LRU caching of partitions in impalad .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 Gerrit-Change-Number: 11208 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 06:03:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3050/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 06:03:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java: http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java@22 PS1, Line 22: import org.apache.impala.authorization.PrivilegeRequestBuilder; unused import http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1972 PS1, Line 1972: test.ok(onServer(true, TPrivilegeLevel.ALL)) : .ok(onServer(true, TPrivilegeLevel.OWNER)) : .ok(onDatabase(true, "functional", TPrivilegeLevel.ALL)) : .ok(onDatabase(true, "functional", TPrivilegeLevel.OWNER)) : .ok(onTable(true, "functional", "alltypes", TPrivilegeLevel.ALL)) : .ok(onTable(true, "functional", "alltypes", TPrivilegeLevel.OWNER)) Should have failure tests for grant option false. http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1985 PS1, Line 1985: nit: This indent is inconsistent with others. See previous test block. http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2188 PS1, Line 2188: test.ok(onServer(true, TPrivilegeLevel.ALL)) : .ok(onServer(true, TPrivilegeLevel.OWNER)) : .ok(onDatabase(true, "functional", TPrivilegeLevel.ALL)) : .ok(onDatabase(true, "functional", TPrivilegeLevel.OWNER)) : .ok(onTable(true, "functional", "alltypes_view", TPrivilegeLevel.ALL)) : .ok(onTable(true, "functional", "alltypes_view", TPrivilegeLevel.OWNER)) Should have error tests for grant option false;. http://gerrit.cloudera.org:8080/#/c/11279/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2223 PS1, Line 2223: .ok(onServer(true, TPrivilegeLevel.ALL)) : .ok(onServer(true, TPrivilegeLevel.OWNER)) : .ok(onDatabase(true, "functional", TPrivilegeLevel.ALL)) : .ok(onDatabase(true, "functional", TPrivilegeLevel.OWNER)) Should have error tests for grant option false;. -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 05:50:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7470: SentryServicePinger logs error messages on success
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11281 ) Change subject: IMPALA-7470: SentryServicePinger logs error messages on success .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/425/ : 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/11281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75782d23c1cb67564a9265bf3cc94fd590c7b666 Gerrit-Change-Number: 11281 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Aug 2018 05:29:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7470: SentryServicePinger logs error messages on success
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11281 ) Change subject: IMPALA-7470: SentryServicePinger logs error messages on success .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/424/ : 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/11281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75782d23c1cb67564a9265bf3cc94fd590c7b666 Gerrit-Change-Number: 11281 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Aug 2018 05:10:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7470: SentryServicePinger logs error messages on success
Fredy Wijaya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11281 ) Change subject: IMPALA-7470: SentryServicePinger logs error messages on success .. IMPALA-7470: SentryServicePinger logs error messages on success SentryServicePinger checks if Sentry is running by calling a Sentry API to get a list of roles. If Sentry is not yet running, an exception will be thrown. However, the Sentry client implementation will log some error messages when an exception is thrown. For the purpose of SentryServicePinger, this can be too noisy and verbose and may also confuse other developers into thinking it was a failure when starting Sentry. The log messages were muted in IMPALA-6878, however since Impala no longer uses the shaded version of Sentry client in IMPALA-7423, the patch in IMPALA-6878 no longer worked. This patch fixes the muting of Sentry error messages by turning off the log level using the non-shaded version of Sentry Thrift client. Testing: - Manually tested by starting Sentry and did not see any error messages logged into stdout. Change-Id: I75782d23c1cb67564a9265bf3cc94fd590c7b666 --- M fe/src/test/java/org/apache/impala/testutil/SentryServicePinger.java 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/11281/2 -- To view, visit http://gerrit.cloudera.org:8080/11281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I75782d23c1cb67564a9265bf3cc94fd590c7b666 Gerrit-Change-Number: 11281 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7466: Improve readability of describe authorization tests
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11278 )
Change subject: IMPALA-7466: Improve readability of describe authorization tests
..
Patch Set 1:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2694
PS1, Line 2694: private class DescribeOutput {
make it a static class
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2695
PS1, Line 2695: private String[] excludedStrings_;
: private String[] includedStrings_;
initialize these two with empty arrays so there's no need for null checks
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2697
PS1, Line 2697: private TDescribeOutputStyle outputStyle_;
make it final
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2703
PS1, Line 2703:
nit: no space
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2703
PS1, Line 2703: public DescribeOutput excludeStrings(String [] excluded) {
add javadoc
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2708
PS1, Line 2708:
nit: no space
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2708
PS1, Line 2708: public DescribeOutput includeStrings(String [] included) {
add javadoc
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2721
PS1, Line 2721:
nit: no space for consistency with the rest of formatting in this file
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2728
PS1, Line 2728:
nit: no space for consistency with the rest of formatting in this file
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2736
PS1, Line 2736: private DescribeOutput describeOutput(TDescribeOutputStyle
style) {
can be made static
http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2820
PS1, Line 2820:
nit: fix indentation
--
To view, visit http://gerrit.cloudera.org:8080/11278
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6244b6dbafbd3827d488588a16e66dbf15d0e115
Gerrit-Change-Number: 11278
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 21 Aug 2018 04:42:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7470: SentryServicePinger logs error messages on success
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11281 Change subject: IMPALA-7470: SentryServicePinger logs error messages on success .. IMPALA-7470: SentryServicePinger logs error messages on success SentryServicePinger checks if Sentry is running by calling a Sentry API to get a list of roles. If Sentry is not yet running, an exception will be thrown. However, the Sentry client implementation will log some error messages when an exception is thrown. For the purpose of SentryServicePinger, this can be too noisy and verbose and may also confuse other developers into thinking it was a failure when starting Sentry. This patch mutes the logging of Sentry error messages when calling a Sentry API to get a list of roles. Testing: - Manually tested by starting Sentry and did not see any error messages logged into stdout. Change-Id: I75782d23c1cb67564a9265bf3cc94fd590c7b666 --- M fe/src/test/java/org/apache/impala/testutil/SentryServicePinger.java 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/11281/1 -- To view, visit http://gerrit.cloudera.org:8080/11281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I75782d23c1cb67564a9265bf3cc94fd590c7b666 Gerrit-Change-Number: 11281 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3047/ -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Aug 2018 03:39:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/statestore/statestore-subscriber.cc@46 PS2, Line 46: using std::string; > hrm, isn't that against the style guide, though? you aren't supposed to hav Right, common/names.h is the one exception since it's just a shortcut to pull in a bunch of common using declarations into a .cc file. Including common/names.h in another header would be a big no-no. -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 03:11:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11280 ) Change subject: WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/423/ : 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/11280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I615f9e6bd167b36cd8d93da59426dd6813ae4984 Gerrit-Change-Number: 11280 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 03:10:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11208 ) Change subject: IMPALA-7437. LRU caching of partitions in impalad .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/421/ : 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/11208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 Gerrit-Change-Number: 11208 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 02:42:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3049/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 02:42:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11208 ) Change subject: IMPALA-7437. LRU caching of partitions in impalad .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3048/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 Gerrit-Change-Number: 11208 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 02:42:18 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/11280 )
Change subject: WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic
updates
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11280/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/11280/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@255
PS1, Line 255: LOG.trace("Request for DB metadata for {}: {}", dbName,
hit.getRef() ? "hit":"miss");
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/11280/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@326
PS1, Line 326: dbName, tableName, resp.table_info.hms_table,
resp.object_version_number);
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/11280
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I615f9e6bd167b36cd8d93da59426dd6813ae4984
Gerrit-Change-Number: 11280
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 21 Aug 2018 02:40:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates
Hello Bharath Vissapragada, Tianyi Wang, Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11280 to review the following change. Change subject: WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates .. WIP: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates This implements cache invalidation inside CatalogdMetaProvider. The design is as follows: - when the catalogd collects updates into the statestore topic, it now adds an additional entry for each table and database. These additional entries are minimal - they only include the object's name, but no metadata. - the old-style topic entries are prefixed with a '1:' whereas the new minimal entries are prefixed with a '2:'. The impalad will subscribe to one or the other prefix depending on whether it is running with --use_local_catalog. Thus, old impalads will not be confused by the new entries and vice versa. - when the impalad gets these topic updates, it forwards them through to the catalog implementation. The LocalCatalog implementation forwards them to the CatalogdMetaProvider, which uses them to invalidate cached metadata as appropriate. This patch includes some basic unit tests. I also did some manual testing by connecting to different impalads and verifying that a session connected to impalad #1 saw the effects of DDLs made by impalad #2 within a short period of time (the statestore topic update frequency). This patch is marked as a WIP since I'd like to evaluate the current e2e test coverage and see if we need any new specific tests for cross-impalad consistency. Change-Id: I615f9e6bd167b36cd8d93da59426dd6813ae4984 --- M be/src/service/impala-server.cc M common/thrift/CatalogObjects.thrift M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/common/Pair.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java 9 files changed, 404 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/11280/1 -- To view, visit http://gerrit.cloudera.org:8080/11280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I615f9e6bd167b36cd8d93da59426dd6813ae4984 Gerrit-Change-Number: 11280 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/422/ : 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/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 02:34:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Hello Bharath Vissapragada, Tianyi Wang, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11208 to look at the new patch set (#7). Change subject: IMPALA-7437. LRU caching of partitions in impalad .. IMPALA-7437. LRU caching of partitions in impalad This changes the CatalogdMetaProvider to use a Guava-based LRU cache. The eviction strategy is currently time-based (1 hour), and it only performs caching of some basic items like partition information, the null-partition-key-value, and table column statistics. It does not cache the table entries themselves, which means that we don't need to do any invalidation propagation via the statestore quite yet. Instead, every query will do an initial fetch of the table metadata in order to know the current version number. That version number is then used as part of the cache key for all further metadata, so when the version number changes, all of the prior cache entries become "unreachable" and effectively evicted. Initially, I attempted to implement this by adding a new MetaProvider implementation that would transparently wrap another MetaProvider implementation (either catalogd-based or direct-from-source). However, I found that I wanted to use catalogd-based implementation details like the version number in the cache key, and trying to abstract this behind an interface wasn't very clear. So, I elected to just embed the caching logic into the CatalogdMetaProvider itself. Note that this patch upgrades the Guava reference in the pom from 11.0.2 to 14.0.1. In fact, I found that Guava 14.0.1 was already leaking onto the classpath by being included in hive-exec.jar, so it was ending up picking one or the other in a somewhat unpredictable fashion. The CacheBuilder class had a small API change between v11 and v14 so I needed to ensure a specific version so that Eclipse and Maven agreed on which version to build against. This includes some basic unit testing and I also verified that some query tests like TPCH pass. Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 --- M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java A fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java M impala-parent/pom.xml 6 files changed, 538 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11208/7 -- To view, visit http://gerrit.cloudera.org:8080/11208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 Gerrit-Change-Number: 11208 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Hello Tim Armstrong, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11253 to look at the new patch set (#3). Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. IMPALA-7457. statestore: allow filtering by key prefix This adds the ability for a statestore subscriber to specify a key prefix which acts as a filter. Only topic entries which match the specified prefix are transmitted to the subscriber. This patch makes use of the new feature for a small optimization: the catalogd subscribes to the catalog topic with a key prefix "!" which we know doesn't match any actual topic items. This avoids the statestore having to reflect back the catalog contents to the catalogd, since the catalogd ignored this info anyway. A later patch will make use of this to publish lightweight catalog object version numbers in the same topic as the catalog objects themselves. The modification to catalogd's topic subscription is covered by existing tests. A new specific test is added to verify the filtering mechanism. Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 --- M be/src/catalog/catalog-server.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/scheduler.cc M be/src/service/impala-server.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M common/thrift/StatestoreService.thrift M tests/statestore/test_statestore.py 10 files changed, 115 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/11253/3 -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/11208 )
Change subject: IMPALA-7437. LRU caching of partitions in impalad
..
Patch Set 6:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@280
PS5, Line 280: // Fetch and re-add those missing ones.
> perhaps add a comment here about this being the miss handler.
Done
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@660
PS5, Line 660:*
> nit: add newline to be consistent with other classes.
Done
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
File
fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java:
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@69
PS5, Line 69:
> perhaps add the suffix "ByRefs" so its clearer that this test is aimed at t
Done
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@71
PS5, Line 71: public void testCachePartitionList() throws Exception {
> clearer to call this partialRefs
Done
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@95
PS5, Line 95: Map partMap =
provider_.loadPartitionsByRefs(
> add a test for loadTableColumnStatistics
did this in r6
--
To view, visit http://gerrit.cloudera.org:8080/11208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5
Gerrit-Change-Number: 11208
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 21 Aug 2018 01:57:18 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11208 ) Change subject: IMPALA-7437. LRU caching of partitions in impalad .. Patch Set 6: (6 comments) oops, I forgot to publish these comments. I see there are more comments I need to address, but posting these older replies for now. http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@268 PS5, Line 268: le, colNam > yea, I actually ended up implementing the negative cache in a later patch i moved the "negative caching" into this patch. The trace message now reports positive hit, negative hit, and missing (total adds up to all of the columns) http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@289 PS5, Line 289: ats > with req, resp, and ret all looking fairly similar, perhaps call this one " Done http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@423 PS5, Line 423: > nit: inconsistent spacing. I'm just going with file consistency-- I've comp Apparently no space is more common in the codebase. I'll make this file consistent. (env)todd@va1022:/data/1/todd/impala$ git grep 'for.*(.* : .*)' fe | wc -l 220 (env)todd@va1022:/data/1/todd/impala$ git grep 'for.*(.*[^ ]: .*)' fe | wc -l 1060 http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@567 PS5, Line 567: } > nit: add a "TableMetaRef: " to make it easier to know that we're looking at Done http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@596 PS5, Line 596: */ > nit: add a newline Done http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@376 PS5, Line 376: should > nit: should Done -- To view, visit http://gerrit.cloudera.org:8080/11208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 Gerrit-Change-Number: 11208 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 01:45:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/catalog/catalog-server.cc@211 PS2, Line 211: string filter_prefix = "!"; > doesn't seem to be used. aha, woops... got lost in a conflict resolution I think. thanks. http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/statestore/statestore-subscriber.cc@46 PS2, Line 46: using std::string; > This should be pulled in automatically by common/names.h hrm, isn't that against the style guide, though? you aren't supposed to have 'using' in header files -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 01:44:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7448: Invalidate recently unused tables from catalogd
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/11224 )
Change subject: IMPALA-7448: Invalidate recently unused tables from catalogd
..
Patch Set 2:
(46 comments)
http://gerrit.cloudera.org:8080/#/c/11224/1/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/11224/1/be/src/catalog/catalog-server.cc@537
PS1, Line 537:
> How do you measure the length of a string literal without using an array?
I mean:
static const char* kMsg = "unused_table_ttl_sec not specified";
Value value(kMsg, strlen(kMsg);
Even though strlen looks like a function call here, gcc -O1 or higher will
optimize out the strlen to just the known constant size, see
https://godbolt.org/z/3AMhwL
http://gerrit.cloudera.org:8080/#/c/11224/2/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
http://gerrit.cloudera.org:8080/#/c/11224/2/be/src/common/global-flags.cc@208
PS2, Line 208: Talbe
typo: Table
nit: I think it's better to avoid talking too much about implementation
mechanism in the doc here. How about something like:
If a table has not been referenced in a query for more than the configured
amount of time, the catalog server will automatically evict its cached metadata
about this table. This has the same effect as a user-initiated "INVALIDATE
METADATA" statement on the unused table. Configuring this to 0 disables
automatic invalidation of tables.
http://gerrit.cloudera.org:8080/#/c/11224/2/be/src/common/global-flags.cc@207
PS2, Line 207: DEFINE_int32(unused_table_ttl_sec, 0, "Catalogd invalidates
unused tables older than this"
: "threshold. Talbe usage is reported periodically from
impalad to catalogd. Then"
: "catalog invalidates old unused tables as if they are
invalidated by a user."
: "0 disables this feature.");
:
: DEFINE_bool(invalidate_tables_on_memory_pressure, false,
"Catalog invalidates recently"
: "unused tables when the old GC generation is almost full.");
nit: missing extra spaces at end of each line before the end quote
http://gerrit.cloudera.org:8080/#/c/11224/2/be/src/exec/catalog-op-executor.h
File be/src/exec/catalog-op-executor.h:
http://gerrit.cloudera.org:8080/#/c/11224/2/be/src/exec/catalog-op-executor.h@72
PS2, Line 72: Status ReportTableUsage(const TReportTableUsageRequest& req,
nit: add some docs
http://gerrit.cloudera.org:8080/#/c/11224/2/common/thrift/CatalogService.thrift
File common/thrift/CatalogService.thrift:
http://gerrit.cloudera.org:8080/#/c/11224/2/common/thrift/CatalogService.thrift@415
PS2, Line 415: num_usage
nit: I think "usage_count" or "num_usages" would be better
http://gerrit.cloudera.org:8080/#/c/11224/2/common/thrift/CatalogService.thrift@419
PS2, Line 419: 1: required map
tables;
nit: extra space in here
http://gerrit.cloudera.org:8080/#/c/11224/2/common/thrift/CatalogService.thrift@423
PS2, Line 423:
nit: no need for empty line
http://gerrit.cloudera.org:8080/#/c/11224/2/common/thrift/CatalogService.thrift@461
PS2, Line 461: TReportTableUsageResponse ReportTableUsage(1:
TReportTableUsageRequest req);
nit: doc
http://gerrit.cloudera.org:8080/#/c/11224/2/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:
http://gerrit.cloudera.org:8080/#/c/11224/2/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@141
PS2, Line 141: if (ImpaladTableUsageTracker.INSTANCE != null) {
: ImpaladTableUsageTracker.INSTANCE.addTables(tbls);
: }
I think we need to do this both on the views _and_ the expanded tables, not
just the top-level views. Otherwise, if some tables are only accessed via
views, we'll never update their lastUsed time, and they'll get constantly
evicted.
If we do this at the end of the function instead of at the top, then I think
we'll cover both the views and the underlying tables, right?
http://gerrit.cloudera.org:8080/#/c/11224/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/11224/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@255
PS1, Line 255: catalogdTableShrinker_ = new CatalogdTableShrinker(this,
> It's initialization scope like a constructor
hm, maybe it would be simpler to just put this as a local variable inside 'run'?
http://gerrit.cloudera.org:8080/#/c/11224/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/11224/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@252
PS2, Line 252: throw new Illeg
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11182 ) Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 Gerrit-Change-Number: 11182 Gerrit-PatchSet: 9 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 01:26:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7448: Invalidate recently unused tables from catalogd
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11224 ) Change subject: IMPALA-7448: Invalidate recently unused tables from catalogd .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/420/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11224 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib549717abefcffb14d9a3814ee8cf0de8bd49e89 Gerrit-Change-Number: 11224 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Aug 2018 01:23:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/419/ : 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/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 01:08:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/planner/AggregationNode.java File fe/src/main/java/org/apache/impala/planner/AggregationNode.java: http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@277 PS5, Line 277: MURMUR_HASH > Sure, I'd be interested to see what he turned up about the differences, to The JIRA was IMPALA-2281. There's some info in the comments. The crux is that Murmur2 fails some SMHasher tests: https://github.com/rurban/smhasher/blob/master/doc/Murmur2A -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 00:46:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7448: Invalidate recently unused tables from catalogd
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11224 ) Change subject: IMPALA-7448: Invalidate recently unused tables from catalogd .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11224/2/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/11224/2/tests/custom_cluster/test_automatic_invalidation.py@62 PS2, Line 62: flake8: W391 blank line at end of file -- To view, visit http://gerrit.cloudera.org:8080/11224 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib549717abefcffb14d9a3814ee8cf0de8bd49e89 Gerrit-Change-Number: 11224 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Aug 2018 00:29:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7448: Invalidate recently unused tables from catalogd
Tianyi Wang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11224 ) Change subject: IMPALA-7448: Invalidate recently unused tables from catalogd .. IMPALA-7448: Invalidate recently unused tables from catalogd This patch implements an automatic invalidation mechanism in catalogd. There are two invalidation strategies: 1. Periodically the HDFS tables that are not used in a configured period "unused_table_ttl_sec" is invalidated from catalogd. 2. If the old GC generation is more than 70% full, 10% LRU tables are invalidated. This could be enabled by backend flag "invalidate_tables_on_memory_pressure". The table usage is reported by impalad to catalogd when the tables are unsed during planning. Tests on time-based invalidation is added. It is manually verified that the GC callback is called if strings are randomly stuffed into catalogd. Change-Id: Ib549717abefcffb14d9a3814ee8cf0de8bd49e89 --- M be/src/catalog/catalog-server.cc M be/src/catalog/catalog-service-client-wrapper.h M be/src/catalog/catalog-util.cc M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/common/global-flags.cc M be/src/exec/catalog-op-executor.cc M be/src/exec/catalog-op-executor.h M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java A fe/src/main/java/org/apache/impala/catalog/CatalogdTableShrinker.java M fe/src/main/java/org/apache/impala/catalog/Db.java M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java A fe/src/main/java/org/apache/impala/catalog/ImpaladTableUsageTracker.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java A fe/src/test/java/org/apache/impala/catalog/CatalogdTableShrinkerTest.java A tests/custom_cluster/test_automatic_invalidation.py 25 files changed, 584 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/11224/2 -- To view, visit http://gerrit.cloudera.org:8080/11224 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib549717abefcffb14d9a3814ee8cf0de8bd49e89 Gerrit-Change-Number: 11224 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/catalog/catalog-server.cc@211 PS2, Line 211: string filter_prefix = "!"; doesn't seem to be used. -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 00:27:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11253/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11253/2//COMMIT_MSG@13 PS2, Line 13: This patch makes use of the new feature for a small optimization: the : catalogd subscribes to the catalog topic with a key prefix "!" I was expecting to see this in the catalog-server registration... did it change? -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 00:26:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/10771/6/be/src/codegen/gen_ir_descriptions.py File be/src/codegen/gen_ir_descriptions.py: http://gerrit.cloudera.org:8080/#/c/10771/6/be/src/codegen/gen_ir_descriptions.py@55 PS6, Line 55: o flake8: E501 line too long (125 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 00:19:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Thomas Marshall has posted comments on this change. (
http://gerrit.cloudera.org:8080/10771 )
Change subject: IMPALA-110: Support for multiple DISTINCT
..
Patch Set 6:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/10771/5/be/src/exec/streaming-aggregation-node.cc
File be/src/exec/streaming-aggregation-node.cc:
http://gerrit.cloudera.org:8080/#/c/10771/5/be/src/exec/streaming-aggregation-node.cc@158
PS5, Line 158: this
> nit: caps
Done
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@36
PS5, Line 36: * Encapsulates all the information needed to compute a list of
aggregate functions with
> It might be worth explaining how this relates to MultiAggregateInfo - why i
Done
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@38
PS5, Line 38: * A mix of distinct and non-distinct aggregation functions is
allowed as long as all
> nit: mix of
Done
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@205
PS5, Line 205: for (int i = 1; i < distinctAggExprs.size(); ++i) {
> Is this reachable now? Can it be a precondition?
Done
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java:
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java@33
PS5, Line 33: * Base class for AggregateInfo and AnalyticInfo containing the
intermediate and output
> Update?
Still seems correct to me
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@40
PS5, Line 40: * SELECT block including their distributed execution by wrapping
a list of
> Might be worth explaining more directly the relationship to AggregateInfo u
Done
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@366
PS5, Line 366: groupingExprs into a CAS
> typo: transpose
Done
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@521
PS5, Line 521: }
> nit: unnecessary intermediate variable 'result'
Done
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/planner/AggregationNode.java
File fe/src/main/java/org/apache/impala/planner/AggregationNode.java:
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@176
PS5, Line 176: // TODO: If this is the transposition phase, then we can
push conjuncts that
> Is this an important optimisation?
Not sure, I'll see if I can get something mocked up and do some experiments
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@277
PS5, Line 277: MURMUR_HASH
> Tianyi decided to use FastHash for exchanges over murmur because it has sli
Sure, I'd be interested to see what he turned up about the differences, to see
if it would have an impact on this particular use case
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@453
PS5, Line 453: nodeResourceProfile_ = ResourceProfile.noReservation(0);
> Probably not if we're making the decision at the aggregator level.
Done
http://gerrit.cloudera.org:8080/#/c/10771/5/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@519
PS5, Line 519: .setMemEstimateBytes(perInstanceMemEstimate)
> nit: Unnecessary intermediate var.
Done
--
To view, visit http://gerrit.cloudera.org:8080/10771
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4
Gerrit-Change-Number: 10771
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Marshall
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 21 Aug 2018 00:18:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10771 to look at the new patch set (#6). Change subject: IMPALA-110: Support for multiple DISTINCT .. IMPALA-110: Support for multiple DISTINCT This patch adds support for having multiple aggregate functions in a single SELECT block that use DISTINCT over different sets of columns. Planner design: - The existing tree-based plan shape with a two-phased aggregation is maintained. - Existing plans are not changed. - Aggregates are grouped into 'aggregation classes' based on their expressions in the distinct portion which may be empty for non-distinct aggregates. - The aggregation framework is generalized to simultaneously process multiple aggregation classes within the tree-based plan. This process splits the results of different aggregation classes into separate rows, so a final aggregation is needed to transpose the results into the desired form. - Main challenge: Each aggregation class consumes and produces different tuples, so conceptually a union-type of tuples flows through the runtime. The tuple union is represented by a TupleRow with one tuple per aggregation class. Only one tuple in such a TupleRow is non-NULL. - Backend exec nodes in the aggregation plan will be aware of this tuple-union either explicitly in their implementation or by relying on expressions that distinguish the aggregation classes. - To distinguish the aggregation classes, e.g. in hash exchanges, CASE expressions are crafted to hash/group on the appropriate slots. Deferred FE work: - Beautify/condense the long CASE exprs - Push applicable conjuncts into individual aggregators before the transposition step - Added a few testing TODOs to reduce the size of this patch - Decide whether we want to change existing plans to the new model Execution design: - Previous patches separated out aggregation logic from the exec node into Aggregators. This is extended to support multiple Aggregators per node, with different grouping and aggregating functions. - There is a fast path for aggregations with only one aggregator, which leaves the execution essentially unchanged from before. - When there are multiple aggregators, the first aggregation node in the plan replicates its input to each aggregator. The output of this step is rows where only a single tuple is non-null, corresponding to the aggregator that produced the row. - A new expr is introduced, ValidTupleId, which takes one of these rows and returns which tuple is non-null. - For additional aggregation nodes, the input is split apart into 'mini-batches' according to which aggregator the row corresponds to. Testing: - Added analyzer and planner tests - Added end-to-end queries tests - Ran hdfs/core tests Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 --- M be/src/codegen/gen_ir_descriptions.py M be/src/exec/CMakeLists.txt A be/src/exec/aggregation-node-base.cc A be/src/exec/aggregation-node-base.h M be/src/exec/aggregation-node.cc M be/src/exec/aggregation-node.h M be/src/exec/aggregator.cc M be/src/exec/aggregator.h M be/src/exec/exec-node.cc M be/src/exec/grouping-aggregator-ir.cc M be/src/exec/grouping-aggregator.cc M be/src/exec/grouping-aggregator.h M be/src/exec/non-grouping-aggregator.cc M be/src/exec/non-grouping-aggregator.h M be/src/exec/streaming-aggregation-node.cc M be/src/exec/streaming-aggregation-node.h M be/src/exprs/CMakeLists.txt M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/aggregate-functions.h M be/src/exprs/scalar-expr.cc A be/src/exprs/valid-tuple-id.cc A be/src/exprs/valid-tuple-id.h M be/src/runtime/row-batch.h M common/thrift/Exprs.thrift M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java A fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java A fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java M fe/src/main/java/org/apache/impala/catalog/AggregateFunction.java M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java M fe/src/main/java/org/apache/impala/planner/AggregationNode.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M test
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Aug 2018 00:15:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3047/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Aug 2018 00:16:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/417/ : 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/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 00:13:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11208 ) Change subject: IMPALA-7437. LRU caching of partitions in impalad .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/416/ : 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/11208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 Gerrit-Change-Number: 11208 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 00:12:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/11208 )
Change subject: IMPALA-7437. LRU caching of partitions in impalad
..
Patch Set 5:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@235
PS5, Line 235: List ret =
Lists.newArrayListWithCapacity(colNames.size());
> Yea, I was going to defer this and try to add a somewhat generic "request m
makes sense.
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@268
PS5, Line 268: ret.size()
> yea, I actually ended up implementing the negative cache in a later patch i
sg, thx!
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@280
PS5, Line 280: new Callable>() {
perhaps add a comment here about this being the miss handler.
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@660
PS5, Line 660: private final long partId_;
nit: add newline to be consistent with other classes.
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
File
fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java:
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@69
PS5, Line 69: public void testCachePartitions() throws Exception {
perhaps add the suffix "ByRefs" so its clearer that this test is aimed at the
loadPartitionsByRefs method (unless I've misunderstood it).
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@71
PS5, Line 71: List refs = allRefs.subList(3, 8);
clearer to call this partialRefs
http://gerrit.cloudera.org:8080/#/c/11208/5/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@95
PS5, Line 95: }
add a test for loadTableColumnStatistics
--
To view, visit http://gerrit.cloudera.org:8080/11208
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5
Gerrit-Change-Number: 11208
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Tue, 21 Aug 2018 00:10:59 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7433: reduce logging on executors
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11202 ) Change subject: IMPALA-7433: reduce logging on executors .. Patch Set 4: Lars, do you also want to take a look ? -- To view, visit http://gerrit.cloudera.org:8080/11202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff Gerrit-Change-Number: 11202 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Aug 2018 00:06:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/11253/2/be/src/statestore/statestore-subscriber.cc@46 PS2, Line 46: using std::string; This should be pulled in automatically by common/names.h -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 00:05:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11253 ) Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/418/ : 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/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Aug 2018 00:00:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11279 ) Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/415/ : 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/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 23:53:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7457. statestore: allow filtering by key prefix
Hello Tim Armstrong, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11253 to look at the new patch set (#2). Change subject: IMPALA-7457. statestore: allow filtering by key prefix .. IMPALA-7457. statestore: allow filtering by key prefix This adds the ability for a statestore subscriber to specify a key prefix which acts as a filter. Only topic entries which match the specified prefix are transmitted to the subscriber. This patch makes use of the new feature for a small optimization: the catalogd subscribes to the catalog topic with a key prefix "!" which we know doesn't match any actual topic items. This avoids the statestore having to reflect back the catalog contents to the catalogd, since the catalogd ignored this info anyway. A later patch will make use of this to publish lightweight catalog object version numbers in the same topic as the catalog objects themselves. The modification to catalogd's topic subscription is covered by existing tests. A new specific test is added to verify the filtering mechanism. Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 --- M be/src/catalog/catalog-server.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/scheduler.cc M be/src/service/impala-server.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M common/thrift/StatestoreService.thrift M tests/statestore/test_statestore.py 10 files changed, 115 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/11253/2 -- To view, visit http://gerrit.cloudera.org:8080/11253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6ddcf3bfaf16bc3cd1ba01100e948ff142a67620 Gerrit-Change-Number: 11253 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7437. LRU caching of partitions in impalad
Hello Bharath Vissapragada, Tianyi Wang, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11208 to look at the new patch set (#6). Change subject: IMPALA-7437. LRU caching of partitions in impalad .. IMPALA-7437. LRU caching of partitions in impalad This changes the CatalogdMetaProvider to use a Guava-based LRU cache. The eviction strategy is currently time-based (1 hour), and it only performs caching of some basic items like partition information, the null-partition-key-value, and table column statistics. It does not cache the table entries themselves, which means that we don't need to do any invalidation propagation via the statestore quite yet. Instead, every query will do an initial fetch of the table metadata in order to know the current version number. That version number is then used as part of the cache key for all further metadata, so when the version number changes, all of the prior cache entries become "unreachable" and effectively evicted. Initially, I attempted to implement this by adding a new MetaProvider implementation that would transparently wrap another MetaProvider implementation (either catalogd-based or direct-from-source). However, I found that I wanted to use catalogd-based implementation details like the version number in the cache key, and trying to abstract this behind an interface wasn't very clear. So, I elected to just embed the caching logic into the CatalogdMetaProvider itself. Note that this patch upgrades the Guava reference in the pom from 11.0.2 to 14.0.1. In fact, I found that Guava 14.0.1 was already leaking onto the classpath by being included in hive-exec.jar, so it was ending up picking one or the other in a somewhat unpredictable fashion. The CacheBuilder class had a small API change between v11 and v14 so I needed to ensure a specific version so that Eclipse and Maven agreed on which version to build against. This includes some basic unit testing and I also verified that some query tests like TPCH pass. Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 --- M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java A fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java M impala-parent/pom.xml 6 files changed, 533 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11208/6 -- To view, visit http://gerrit.cloudera.org:8080/11208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9a57521ad851da605604a1e7c48d3d6627da5df5 Gerrit-Change-Number: 11208 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 23:20:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Hello Bharath Vissapragada, Tianyi Wang, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11231 to look at the new patch set (#4). Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. IMPALA-7447. Evict LocalCatalog cache entries based on size This pulls in the 'sizeof' library from ehcache (Apache-licensed) and uses it to implement size-based eviction of cache entries in LocalCatalog. This is difficult to test without being quite fragile to small changes in the cached structures. However, I added a simple unit test as a general sanity-check that it is computing some reasonable result. Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 --- M be/src/runtime/exec-env.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/pom.xml M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java 7 files changed, 111 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/11231/4 -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11231/3/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java File fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java: http://gerrit.cloudera.org:8080/#/c/11231/3/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@111 PS3, Line 111: // the size loosely. > nm the last point about sizes... saw that the other change (other tests in There is a way to fake timing but I didn't want to duplicate the testing of the cache itself (ie I could advance time and make sure I get a miss on something where I previously got a hit, but doesnt seem too interesting). Is there a particular behavior on expiration that you're worried about? -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 23:19:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11279 Change subject: IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements .. IMPALA-7344: Restrict ALTER DATABASE/TABLE SET OWNER statements Prior to this patch, any user with ALTER privilege could alter the database/table ownership from one user/role to another user/role. This is undesirable because altering an object ownership means giving a full access to that object. This patch restricts the ALTER DATABASE/TABLE SET OWNER statements to require ALL/OWNER with GRANT OPTION when authorization is enabled. Testing: - Added FE authorization tests - Ran all FE tests - Ran core tests Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 --- M bin/impala-config.sh M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 14 files changed, 229 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/11279/1 -- To view, visit http://gerrit.cloudera.org:8080/11279 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2485933c02b5384950b7c882ba1eb0fd703db5a3 Gerrit-Change-Number: 11279 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-7447. Evict LocalCatalog cache entries based on size
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11231 ) Change subject: IMPALA-7447. Evict LocalCatalog cache entries based on size .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/11231/3/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/11231/3/be/src/runtime/exec-env.cc@90 PS3, Line 90: > nit: get rid of extra spaces (here and several words to the right) Done http://gerrit.cloudera.org:8080/#/c/11231/3/common/thrift/BackendGflags.thrift File common/thrift/BackendGflags.thrift: http://gerrit.cloudera.org:8080/#/c/11231/3/common/thrift/BackendGflags.thrift@87 PS3, Line 87: seconds > for seconds, seems we just use "s" as a suffix. lets go with that for now f Done -- To view, visit http://gerrit.cloudera.org:8080/11231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96af49b35c17e505b7b6785e78d140939085d91 Gerrit-Change-Number: 11231 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 23:14:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11182 ) Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/11182/9/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11182/9/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java@63 PS9, Line 63: List loadPartitionList(TableMetaRef table) > pls add a comment or point from here to PartitionRef, which I see has more k, added to the LRU eviction patch. -- To view, visit http://gerrit.cloudera.org:8080/11182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 Gerrit-Change-Number: 11182 Gerrit-PatchSet: 9 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 23:07:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11182 ) Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/414/ : 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/11182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 Gerrit-Change-Number: 11182 Gerrit-PatchSet: 9 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 22:54:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11182 ) Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/11182/9/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11182/9/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java@63 PS9, Line 63: List loadPartitionList(TableMetaRef table) pls add a comment or point from here to PartitionRef, which I see has more info. just looked here from the lru patch so would be more convenient to have the comment here. can make the change in the lru patch to let this one continue building. -- To view, visit http://gerrit.cloudera.org:8080/11182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 Gerrit-Change-Number: 11182 Gerrit-PatchSet: 9 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 22:46:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/413/ : 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/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 22:46:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10847 ) Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@179 PS10, Line 179: : nit: space on wrong side of : You could also simplify to "Sender UID: root, PID: 1234" http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297 PS10, Line 297: sigaction(SIGTERM, &action, nullptr); We should at least check the return value here. We also should pass an empty struct sigaction *oldact and verify that there was no other signal handler present. http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/util/minidump.cc File be/src/util/minidump.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/util/minidump.cc@102 PS10, Line 102: : nit: whitespace, see comment on other location http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/util/minidump.cc@119 PS10, Line 119: sigaction(SIGUSR1, &sig_action, NULL); While you're here, maybe check the return value to make sure this was successful, and pass struct sigaction *oldact to make sure we didn't overwrite any other handlers? -- To view, visit http://gerrit.cloudera.org:8080/10847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 10 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 20 Aug 2018 22:41:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11182 ) Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/412/ : 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/11182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 Gerrit-Change-Number: 11182 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 22:37:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7466: Improve readability of describe authorization tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11278 ) Change subject: IMPALA-7466: Improve readability of describe authorization tests .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/411/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11278 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6244b6dbafbd3827d488588a16e66dbf15d0e115 Gerrit-Change-Number: 11278 Gerrit-PatchSet: 1 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 22:15:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Hello Bharath Vissapragada, Tianyi Wang, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11182 to look at the new patch set (#9). Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. IMPALA-7436: initial fetch-from-catalogd implementation This patch adds a new RPC to the catalogd which allows a client to fetch a partial view of table or database metadata. Various subsets of information can be specified and are sent back in fairly "raw" format. A new MetaProvider implementation is added which uses this API to support granular fetching of metadata into the impalad. The interface had to be reworked in a few ways to support this: - This API uses partition IDs instead of names to specify them. So, the listPartitions API now returns opaque PartitionRefs which are passed back to the MetaProvider when loading more partition details. The new implementation stores the IDs in these refs while the direct-to-HMS implementation just uses names. - The fetching of file descriptors was merged into the loading of other partition metadata. I couldn't think of any cases where we needed to list partition details without also fetching the file descriptors so it simplified things a bit to merge the two. This was a lot easier to implement for CatalogdMetaProvider since the file metadata is stored by partition rather than looked up by a directory as in the previous API. This necessitated moving some of the logic out of LocalFsTable into DirectMetaProvider, so LocalFsTable no longer deals directly with HDFS APIs like FileStatus. - The handling of "default partition" for an unpartitioned table moved into the MetaProvider implementations itself instead of LocalFsTable. This is because the CatalogdProvider sees the "default partition" as a partition that actually has an identifier on the catalogd, whereas the DirectMetaProvider does not. So, now both providers export the "default partition" as a partition like all the others. This patch also starts to address one of the potential semantic risks of partial caching on the impalad. If one query fetches some subset of partitions, then a DDL occurs to change the table metadata, and another query is submitted, we want to ensure that the metadata for the latter query still reads a consistent snapshot. In other words, we need to ensure that the metadata like partition list and table schema come from the same snapshot as the finer-grained metadata like partition contents. In order to implement this, the MetadataProvider API now requires that callers use a 'TableRef' object to specify the table to be read, instead of the dbName/tableName. In the DirectMetaProvider we don't have any convenient version numbers for a table, so the TableRef just encapsulates the naming. In the CatalogdMetaProvider, we additionally store the version number of the table, and then all subsequent requests verify that the version number has not changed. If it detects a concurrent modification, an exception is thrown. In a future patch, I'm planning on having the frontend catch the exception and trigger a "re-plan". Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 --- M be/src/catalog/catalog-server.cc M be/src/catalog/catalog-service-client-wrapper.h M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/exec/catalog-op-executor.cc M be/src/exec/catalog-op-executor.h M be/src/service/fe-support.cc M common/fbs/CMakeLists.txt M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java M fe/src/main/java/org/apache/impala/catalog/Db.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java A fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java A fe/src/main/java/org/apache/impala/catalog/local/InconsistentMetadataFetchException.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalHbaseTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalPartitionSpec.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalView.java M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java M fe/src/main/java/org/apache/impala/common/RuntimeE
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11182 ) Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3046/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 Gerrit-Change-Number: 11182 Gerrit-PatchSet: 9 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 22:06:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 3: Code-Review+1 Carry +1 -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 22:05:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h@344 PS2, Line 344: instances_prepare_promise_ > This variable doesn't exist. Fixed. http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h@345 PS2, Line 345: accessor > reader? Since writes by the startup thread are allowed, and in fact require Yes, was following the terminology accessor/mutator. Using the term "readers" make it clearer. http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h@351 PS2, Line 351: accessor > readers Done -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 22:05:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Hello Tim Armstrong, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11270 to look at the new patch set (#3). Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. IMPALA-7422: Fix a race in QueryState::StartFInstances() A recent commit for IMPALA-7163 (cbc8c63) introduced a race between insertion into QueryState::fragment_map_ and the thread creation. In particular, after the aforementioned commit, the counting barrier 'instances_prepared_barrier_' is used for synchronizing callers of Cancel()/PublishFilter() and the PREPARE phase of fragment instances. Cancel()/PublishFilter() cannot proceed until all fragment instances have finished preparing; 'instances_prepared_barrier_' is updated by fragment instances once each of them is done preparing. The race is due to the fact that QueryState::StartFInstances() doesn't insert the fragment instance into 'fragment_map_' until after the fragment instance thread has been spawned. So, it's possible for the newly spawned thread to finish preparing and update the counting barrier before the insertion into 'fragment_map_' happens. It's therefore possible for PublishFilter() to have gotten unblocked before a fragment is inserted into 'fragment_map_', triggering the DCHECK() in IMPALA-7422. This change fixes the race by moving the insertion into fragment_map_ before the thread is spawned. Testing done: Exhaustive debug + release builds which previously ran into this race Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e --- M be/src/runtime/query-state.cc M be/src/runtime/query-state.h 2 files changed, 14 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/11270/3 -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11182 ) Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. Patch Set 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@314 PS8, Line 314: @Immutable Had to add this annotation to make errorprone happy after rebasing on d29300281b5d07c1ed98032536c008b076a7baa5 http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@311 PS8, Line 311: @Immutable this one too http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java@106 PS8, Line 106: @Immutable this one too http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java File fe/src/main/java/org/apache/impala/common/RuntimeEnv.java: http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/main/java/org/apache/impala/common/RuntimeEnv.java@46 PS8, Line 46: isTestEnv_ = false; Needed to add this in order to fix test runs. Without this, in 'mvn test', tests that run earlier would pollute tests that ran later. Specifically, the LocalCatalogTests ended up seeing some different behavior depending on whether this was set or not. http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java File fe/src/test/java/org/apache/impala/common/FrontendTestBase.java: http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java@100 PS8, Line 100: RuntimeEnv.INSTANCE.reset(); fixed this cleanup to reset fully http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/test/java/org/apache/impala/planner/PlannerTest.java File fe/src/test/java/org/apache/impala/planner/PlannerTest.java: http://gerrit.cloudera.org:8080/#/c/11182/8/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@532 PS8, Line 532: Assert.assertTrue(RuntimeEnv.INSTANCE.isTestEnv()); oops, didn't mean to add this. Will remove. -- To view, visit http://gerrit.cloudera.org:8080/11182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 Gerrit-Change-Number: 11182 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Aug 2018 22:02:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7436: initial fetch-from-catalogd implementation
Hello Bharath Vissapragada, Tianyi Wang, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11182 to look at the new patch set (#8). Change subject: IMPALA-7436: initial fetch-from-catalogd implementation .. IMPALA-7436: initial fetch-from-catalogd implementation This patch adds a new RPC to the catalogd which allows a client to fetch a partial view of table or database metadata. Various subsets of information can be specified and are sent back in fairly "raw" format. A new MetaProvider implementation is added which uses this API to support granular fetching of metadata into the impalad. The interface had to be reworked in a few ways to support this: - This API uses partition IDs instead of names to specify them. So, the listPartitions API now returns opaque PartitionRefs which are passed back to the MetaProvider when loading more partition details. The new implementation stores the IDs in these refs while the direct-to-HMS implementation just uses names. - The fetching of file descriptors was merged into the loading of other partition metadata. I couldn't think of any cases where we needed to list partition details without also fetching the file descriptors so it simplified things a bit to merge the two. This was a lot easier to implement for CatalogdMetaProvider since the file metadata is stored by partition rather than looked up by a directory as in the previous API. This necessitated moving some of the logic out of LocalFsTable into DirectMetaProvider, so LocalFsTable no longer deals directly with HDFS APIs like FileStatus. - The handling of "default partition" for an unpartitioned table moved into the MetaProvider implementations itself instead of LocalFsTable. This is because the CatalogdProvider sees the "default partition" as a partition that actually has an identifier on the catalogd, whereas the DirectMetaProvider does not. So, now both providers export the "default partition" as a partition like all the others. This patch also starts to address one of the potential semantic risks of partial caching on the impalad. If one query fetches some subset of partitions, then a DDL occurs to change the table metadata, and another query is submitted, we want to ensure that the metadata for the latter query still reads a consistent snapshot. In other words, we need to ensure that the metadata like partition list and table schema come from the same snapshot as the finer-grained metadata like partition contents. In order to implement this, the MetadataProvider API now requires that callers use a 'TableRef' object to specify the table to be read, instead of the dbName/tableName. In the DirectMetaProvider we don't have any convenient version numbers for a table, so the TableRef just encapsulates the naming. In the CatalogdMetaProvider, we additionally store the version number of the table, and then all subsequent requests verify that the version number has not changed. If it detects a concurrent modification, an exception is thrown. In a future patch, I'm planning on having the frontend catch the exception and trigger a "re-plan". Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 --- M be/src/catalog/catalog-server.cc M be/src/catalog/catalog-service-client-wrapper.h M be/src/catalog/catalog.cc M be/src/catalog/catalog.h M be/src/exec/catalog-op-executor.cc M be/src/exec/catalog-op-executor.h M be/src/service/fe-support.cc M common/fbs/CMakeLists.txt M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java M fe/src/main/java/org/apache/impala/catalog/Db.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java A fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java A fe/src/main/java/org/apache/impala/catalog/local/InconsistentMetadataFetchException.java M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalHbaseTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalPartitionSpec.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalView.java M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java M fe/src/main/java/org/apache/impala/common/RuntimeE
[Impala-ASF-CR] IMPALA-7433: reduce logging on executors
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11202 ) Change subject: IMPALA-7433: reduce logging on executors .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-state.cc@356 PS3, Line 356: VLOG(2) > Yes, please do add logging for errors in QueryState::Init(). I'm logging the returned status in ImpalaInternalService::ExecQueryFInstances() as a kind of catch-all -- To view, visit http://gerrit.cloudera.org:8080/11202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff Gerrit-Change-Number: 11202 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 21:57:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7433: reduce logging on executors
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11202 ) Change subject: IMPALA-7433: reduce logging on executors .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-state.cc@356 PS3, Line 356: VLOG(2) > We do already have log messages before and after this. I might be missing s Yes, please do add logging for errors in QueryState::Init(). -- To view, visit http://gerrit.cloudera.org:8080/11202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff Gerrit-Change-Number: 11202 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 21:54:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7449: Fix network throughput calculation of DataStreamSender
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11241 ) Change subject: IMPALA-7449: Fix network throughput calculation of DataStreamSender .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11241 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I344ac76c0a1a49b4da3d37d2c547f3d5051ebe24 Gerrit-Change-Number: 11241 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Mon, 20 Aug 2018 21:49:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6923: Remove create database.py and perf result datastore.py
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Remove create_database.py and perf_result_datastore.py .. Patch Set 22: > Patch Set 22: > > Build Failed > > https://jenkins.impala.io/job/gerrit-code-review-checks/410/ : Initial code > review checks failed. See linked job for details on the failure. Nithya, I'd recommend closing this review, rebasing against the latest upstream changes, and reposting as a new review. -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 22 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan Gerrit-Comment-Date: Mon, 20 Aug 2018 21:47:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7433: reduce logging on executors
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11202 ) Change subject: IMPALA-7433: reduce logging on executors .. Patch Set 4: Code-Review+1 Thanks for the explanation. -- To view, visit http://gerrit.cloudera.org:8080/11202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff Gerrit-Change-Number: 11202 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 21:45:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6923: Remove create database.py and perf result datastore.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Remove create_database.py and perf_result_datastore.py .. Patch Set 22: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/410/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 22 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan Gerrit-Comment-Date: Mon, 20 Aug 2018 21:43:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7466: Improve readability of describe authorization tests
Adam Holley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11278 Change subject: IMPALA-7466: Improve readability of describe authorization tests .. IMPALA-7466: Improve readability of describe authorization tests This patch improves the readability and usability of the describe authorization tests. It removes the ambiguity of the function parameters required for validating the describe output. Testing: - Refactored describe authorization tests - Ran AuthorizationStmtTests Change-Id: I6244b6dbafbd3827d488588a16e66dbf15d0e115 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 1 file changed, 105 insertions(+), 61 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/11278/1 -- To view, visit http://gerrit.cloudera.org:8080/11278 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6244b6dbafbd3827d488588a16e66dbf15d0e115 Gerrit-Change-Number: 11278 Gerrit-PatchSet: 1 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Fredy Wijaya
[Impala-ASF-CR] IMPALA-7433: reduce logging on executors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11202 ) Change subject: IMPALA-7433: reduce logging on executors .. Patch Set 4: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/409/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff Gerrit-Change-Number: 11202 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 21:37:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/408/ : 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/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 21:17:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6923: Remove create database.py and perf result datastore.py
Nithya Janarthanan has uploaded a new patch set (#22). ( http://gerrit.cloudera.org:8080/10100 ) Change subject: IMPALA-6923: Remove create_database.py and perf_result_datastore.py .. IMPALA-6923: Remove create_database.py and perf_result_datastore.py Description: The current upstream usage indicates that report_benchmark_results.py script is the only script in this folder that is used to generate a report comparing performance between two given runs of the performance tests. The other two scripts create_database.py and perf_result_datastore.py store some metrics from a performance test run to database on a specified impala instance. However they rely on some Internal resources not available to external apache community to generate a meaningful interpretation/report of the metrics stored in the performance database. This change also removes lines of code from report_benchmark_results.py that were previously parsing command line options, that were being used by the two scripts that are being removed. Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c --- D tests/benchmark/create_database.py D tests/benchmark/perf_result_datastore.py M tests/benchmark/report_benchmark_results.py 3 files changed, 15 insertions(+), 417 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/10100/22 -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 22 Gerrit-Owner: Nithya Janarthanan Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Nithya Janarthanan
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 2: (1 comment) Thanks for the explanation. Seems good aside from a stale comment. http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h@344 PS2, Line 344: instances_prepare_promise_ This variable doesn't exist. -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 21:06:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7433: reduce logging on executors
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11202 ) Change subject: IMPALA-7433: reduce logging on executors .. Patch Set 3: (2 comments) I'm open to reinstating some logs if there's a clear scenario where we need them, but I think we need to remove some of this "just in case" logging to reduce the signal-to-noise ratio. If there are specific error conditions we're concerned about maybe we can add targeted logging that will only be triggered if we're in an interesting case. http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-exec-mgr.cc File be/src/runtime/query-exec-mgr.cc: http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-exec-mgr.cc@155 PS3, Line 155: VLOG(2) > This may be useful for debugging issue such as IMPALA-7194. Not sure if the We separately log when each fragment instance completes, so this additional log doesn't provide that much information beyond this (aside from the refcnt and the fact that it made it slightly further along in the teardown path). I0813 12:10:50.417552 31256 query-state.cc:485] Instance completed. instance_id=fd4ae28bc993236e:27343be10006 #in-flight=0 status=OK http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-state.cc@356 PS3, Line 356: VLOG(2) > This one may be useful to keep to indicate whether we manage to get to the We do already have log messages before and after this. I might be missing something but I don't see many things that can go wrong in that window. There are a couple of error paths for thread creation and QueryState::Init() where the status gets propagated back to the caller. I'll add logging of those errors actually. I0813 12:10:50.249850 31250 impala-internal-service.cc:49] ExecQueryFInstances(): query_id=fd4ae28bc993236e:27343be1 coord=tarmstrong-box:22000 #instances=2 I0813 12:10:50.250722 31256 query-state.cc:477] Executing instance. instance_id=fd4ae28bc993236e:27343be10006 fragment_idx=1 per_fragment_instance_idx=2 coord_state_idx=1 #in-flight=1 -- To view, visit http://gerrit.cloudera.org:8080/11202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff Gerrit-Change-Number: 11202 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 21:00:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7433: reduce logging on executors
Hello Michael Ho, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11202 to look at the new patch set (#4). Change subject: IMPALA-7433: reduce logging on executors .. IMPALA-7433: reduce logging on executors Moved logs to -v=2 for reasons described in the JIRA. Added more details to some existing log messages or new less-frequent log messages so that useful information is not removed. Sample logging for an executor after the change: I0813 12:10:50.249850 31250 impala-internal-service.cc:49] ExecQueryFInstances(): query_id=fd4ae28bc993236e:27343be1 coord=tarmstrong-box:22000 #instances=2 I0813 12:10:50.250722 31256 query-state.cc:477] Executing instance. instance_id=fd4ae28bc993236e:27343be10006 fragment_idx=1 per_fragment_instance_idx=2 coord_state_idx=1 #in-flight=1 I0813 12:10:50.250804 31259 query-state.cc:477] Executing instance. instance_id=fd4ae28bc993236e:27343be10003 fragment_idx=2 per_fragment_instance_idx=2 coord_state_idx=1 #in-flight=2 I0813 12:10:50.374167 31259 query-state.cc:485] Instance completed. instance_id=fd4ae28bc993236e:27343be10003 #in-flight=1 status=OK I0813 12:10:50.375370 31269 krpc-data-stream-mgr.cc:294] DeregisterRecvr(): fragment_instance_id=fd4ae28bc993236e:27343be10006, node=3 I0813 12:10:50.417552 31256 query-state.cc:485] Instance completed. instance_id=fd4ae28bc993236e:27343be10006 #in-flight=0 status=OK I0813 12:10:50.418007 31256 query-exec-mgr.cc:179] ReleaseQueryState(): deleted query_id=fd4ae28bc993236e:27343be1 Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff --- M be/src/exec/scan-node.cc M be/src/runtime/initial-reservations.cc M be/src/runtime/krpc-data-stream-recvr.cc M be/src/runtime/mem-tracker.cc M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/runtime-filter-bank.cc M be/src/service/impala-internal-service.cc 8 files changed, 36 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/11202/4 -- To view, visit http://gerrit.cloudera.org:8080/11202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff Gerrit-Change-Number: 11202 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 2: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h@345 PS2, Line 345: accessor reader? Since writes by the startup thread are allowed, and in fact required to be, earlier, right? http://gerrit.cloudera.org:8080/#/c/11270/2/be/src/runtime/query-state.h@351 PS2, Line 351: accessor readers -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 20:46:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11270/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11270/1//COMMIT_MSG@14 PS1, Line 14: cnanot > nit: type Done http://gerrit.cloudera.org:8080/#/c/11270/1/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/11270/1/be/src/runtime/query-state.cc@403 PS1, Line 403: // Update fragment_map_. Has to happen before the thread is spawned below or > Isn't a race still possible between this thread modifying the map in a subs 'fragment_map_' is supposed to be modified by the thread which calls StartFInstances(). Other threads may not access 'fragment_map_' until all fragment instances have finished preparing (i.e. they are blocked on WaitForPrepare()). At this point, there is at least one fragment instance which hasn't yet been started so no other thread should be accessing fragment_map_. Added some comments in query-state.h. -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 20:39:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Hello Tim Armstrong, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11270 to look at the new patch set (#2). Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. IMPALA-7422: Fix a race in QueryState::StartFInstances() A recent commit for IMPALA-7163 (cbc8c63) introduced a race between insertion into QueryState::fragment_map_ and the thread creation. In particular, after the aforementioned commit, the counting barrier 'instances_prepared_barrier_' is used for synchronizing callers of Cancel() and PublishFilter() and the PREPARE phase of fragment instances. Cancel() and PublishFilter() cannot proceed until all fragment instances have finished preparing; 'instances_prepared_barrier_' is updated by fragment instances once each of them is done preparing. The race is due to the fact that QueryState::StartFInstances() doesn't insert the fragment instance into 'fragment_map_' until after the fragment instance thread has been spawned. So, it's possible for the newly spawned thread to finish preparing and update the counting barrier before the insertion into 'fragment_map_' happens. It's therefore possible for, PublishFilter() to have gotten unblocked before a fragment is inserted into 'fragment_map_', triggering the DCHECK() in IMPALA-7422. This change fixes the race by moving the insertion into fragment_map_ before the thread is spawned. Testing done: Exhaustive debug + release builds which previously ran into this race Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e --- M be/src/runtime/query-state.cc M be/src/runtime/query-state.h 2 files changed, 13 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/11270/2 -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7453. Intern HdfsStorageDescriptors
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11236 ) Change subject: IMPALA-7453. Intern HdfsStorageDescriptors .. IMPALA-7453. Intern HdfsStorageDescriptors The number of unique HdfsStorageDescriptors in a warehouse is typically much smaller than the number of partitions. Each object takes 32/40 bytes (with/without compressed OOPs respectively). So, by interning these objects, we can save that amount of memory as well as one object per partition. The overall savings aren't huge (on the order of tens of MBs) but the change is pretty simple so seems worthwhile. This patch also pulls in the errorprone annotations into the pom so that errorprone can ensure that the class can be annotated as Immutable. errorprone checks that classes annotated as Immutable only contain immutable fields. I tested this change by comparing 'jmap -histo:live' on a catalogd before/after. For my local dev environment test warehouse, I had 12055 instances (385kb) before the change and 24 instances (768 bytes) after. Change-Id: I9ef93148d629b060fa9f67c631e9c3d904a0ccf9 Reviewed-on: http://gerrit.cloudera.org:8080/11236 Reviewed-by: Bharath Vissapragada Tested-by: Impala Public Jenkins --- M fe/pom.xml M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java M fe/src/main/java/org/apache/impala/catalog/local/LocalKuduTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalPartitionSpec.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java 6 files changed, 86 insertions(+), 26 deletions(-) Approvals: Bharath Vissapragada: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11236 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9ef93148d629b060fa9f67c631e9c3d904a0ccf9 Gerrit-Change-Number: 11236 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7433: reduce logging on executors
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11202 ) Change subject: IMPALA-7433: reduce logging on executors .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-exec-mgr.cc File be/src/runtime/query-exec-mgr.cc: http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-exec-mgr.cc@155 PS3, Line 155: VLOG(2) This may be useful for debugging issue such as IMPALA-7194. Not sure if the newly added line below is sufficient as it may help to see when each fragment instance finishes. http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/11202/3/be/src/runtime/query-state.cc@356 PS3, Line 356: VLOG(2) This one may be useful to keep to indicate whether we manage to get to the point of starting fragment instances after creating the QueryState. While we may consider bumping the log level when problem occurs, this may be a bit hard in practice if the problem is due to a race which doesn't occur consistently. -- To view, visit http://gerrit.cloudera.org:8080/11202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c1db44acc6def2b05a4fd032c63716e08cdf5ff Gerrit-Change-Number: 11202 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Mon, 20 Aug 2018 20:18:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] cleanup: remove RuntimeState::exec env
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11269 ) Change subject: cleanup: remove RuntimeState::exec_env_ .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/407/ : 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/11269 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a1b1bdd41e3d10982b3a4bdb0217e716b4df67f Gerrit-Change-Number: 11269 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 19:58:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7407: Fix test cancellation failure on KeyboardInterrupt
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11255 ) Change subject: IMPALA-7407: Fix test_cancellation failure on KeyboardInterrupt .. Patch Set 6: Code-Review+2 carrying forward -- To view, visit http://gerrit.cloudera.org:8080/11255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I65302ffb838d5185f77853bc2e53296f3a701d93 Gerrit-Change-Number: 11255 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 20 Aug 2018 19:56:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7407: Fix test cancellation failure on KeyboardInterrupt
Thomas Marshall has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11255 ) Change subject: IMPALA-7407: Fix test_cancellation failure on KeyboardInterrupt .. IMPALA-7407: Fix test_cancellation failure on KeyboardInterrupt test_cancellation runs a shell process, executes a query, sleeps, sends a sigint to the process, and then checks that the query is cancelled. If the sigint is sent prior to the shell installing its signal handler, the test can fail with a KeyboardInterrupt. This patch removes the reliance on the sleep being long enough by actually reading the output of the shell and only cancelling the query once the shell shows that it has started running. Testing: - Ran test_cancellation in a loop. Change-Id: I65302ffb838d5185f77853bc2e53296f3a701d93 Reviewed-on: http://gerrit.cloudera.org:8080/11255 Tested-by: Impala Public Jenkins Reviewed-by: Thomas Marshall --- M tests/shell/test_shell_commandline.py M tests/shell/util.py 2 files changed, 17 insertions(+), 1 deletion(-) Approvals: Impala Public Jenkins: Verified Thomas Marshall: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I65302ffb838d5185f77853bc2e53296f3a701d93 Gerrit-Change-Number: 11255 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-7407: Fix test cancellation failure on KeyboardInterrupt
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11255 ) Change subject: IMPALA-7407: Fix test_cancellation failure on KeyboardInterrupt .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I65302ffb838d5185f77853bc2e53296f3a701d93 Gerrit-Change-Number: 11255 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 20 Aug 2018 19:54:09 + Gerrit-HasComments: No
[Impala-ASF-CR] cleanup: remove RuntimeState::exec env
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11269 ) Change subject: cleanup: remove RuntimeState::exec_env_ .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/406/ : 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/11269 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a1b1bdd41e3d10982b3a4bdb0217e716b4df67f Gerrit-Change-Number: 11269 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 19:51:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7422: Fix a race in QueryState::StartFInstances()
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11270 ) Change subject: IMPALA-7422: Fix a race in QueryState::StartFInstances() .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11270/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11270/1//COMMIT_MSG@14 PS1, Line 14: cnanot nit: type http://gerrit.cloudera.org:8080/#/c/11270/1/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/11270/1/be/src/runtime/query-state.cc@403 PS1, Line 403: // Update fragment_map_. Has to happen before the thread is spawned below or Isn't a race still possible between this thread modifying the map in a subsequent iteration of the loop and threads for other fragments accessing the map? Since the map isn't threadsafe. -- To view, visit http://gerrit.cloudera.org:8080/11270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I35f2a5b0ea5143703850ffc229cec0e4294e6a3e Gerrit-Change-Number: 11270 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 19:26:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] cleanup: remove RuntimeState::exec env
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11269 ) Change subject: cleanup: remove RuntimeState::exec_env_ .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11269/2/be/src/exec/partitioned-hash-join-builder.cc File be/src/exec/partitioned-hash-join-builder.cc: http://gerrit.cloudera.org:8080/#/c/11269/2/be/src/exec/partitioned-hash-join-builder.cc@153 PS2, Line 153: ExecEnv::GetInstance()->buffer_pool(), buffer_pool_client_, spillable_buffer_size_)); > line too long (93 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/11269 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a1b1bdd41e3d10982b3a4bdb0217e716b4df67f Gerrit-Change-Number: 11269 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Aug 2018 19:19:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] cleanup: remove RuntimeState::exec env
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11269 to look at the new patch set (#3). Change subject: cleanup: remove RuntimeState::exec_env_ .. cleanup: remove RuntimeState::exec_env_ Change-Id: I4a1b1bdd41e3d10982b3a4bdb0217e716b4df67f --- M be/src/exec/grouping-aggregator.cc M be/src/exec/hbase-scan-node.cc M be/src/exec/hbase-table-writer.cc M be/src/exec/hdfs-orc-scanner.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/kudu-scan-node-base.cc M be/src/exec/kudu-table-sink.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/reservation-manager.cc M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h 14 files changed, 26 insertions(+), 50 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11269/3 -- To view, visit http://gerrit.cloudera.org:8080/11269 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4a1b1bdd41e3d10982b3a4bdb0217e716b4df67f Gerrit-Change-Number: 11269 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] cleanup: remove RuntimeState::exec env
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11269 ) Change subject: cleanup: remove RuntimeState::exec_env_ .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11269/2/be/src/exec/partitioned-hash-join-builder.cc File be/src/exec/partitioned-hash-join-builder.cc: http://gerrit.cloudera.org:8080/#/c/11269/2/be/src/exec/partitioned-hash-join-builder.cc@153 PS2, Line 153: ExecEnv::GetInstance()->buffer_pool(), buffer_pool_client_, spillable_buffer_size_)); line too long (93 > 90) -- To view, visit http://gerrit.cloudera.org:8080/11269 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a1b1bdd41e3d10982b3a4bdb0217e716b4df67f Gerrit-Change-Number: 11269 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:14:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] cleanup: remove RuntimeState::exec env
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11269 Change subject: cleanup: remove RuntimeState::exec_env_ .. cleanup: remove RuntimeState::exec_env_ Change-Id: I4a1b1bdd41e3d10982b3a4bdb0217e716b4df67f --- M be/src/exec/grouping-aggregator.cc M be/src/exec/hbase-scan-node.cc M be/src/exec/hbase-table-writer.cc M be/src/exec/hdfs-orc-scanner.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/kudu-scan-node-base.cc M be/src/exec/kudu-table-sink.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/reservation-manager.cc M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h 14 files changed, 25 insertions(+), 49 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11269/2 -- To view, visit http://gerrit.cloudera.org:8080/11269 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4a1b1bdd41e3d10982b3a4bdb0217e716b4df67f Gerrit-Change-Number: 11269 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
