[Impala-ASF-CR] IMPALA-7973: Add support for fine grained updates at partition level.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13111 ) Change subject: IMPALA-7973: Add support for fine grained updates at partition level. .. Patch Set 2: (11 comments) http://gerrit.cloudera.org:8080/#/c/13111/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13111/2//COMMIT_MSG@7 PS2, Line 7: IMPALA-7973: Add support for fine grained updates at partition level. nit: mention about "events" in the title? Otherwise it looks very general . http://gerrit.cloudera.org:8080/#/c/13111/2//COMMIT_MSG@13 PS2, Line 13: nit: ..affected.. http://gerrit.cloudera.org:8080/#/c/13111/2//COMMIT_MSG@14 PS2, Line 14: HMS processes add/drop partitions : in a transaction Not sure I understand this, can you please clarify? http://gerrit.cloudera.org:8080/#/c/13111/2//COMMIT_MSG@19 PS2, Line 19: insead nit:typo http://gerrit.cloudera.org:8080/#/c/13111/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/13111/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2085 PS2, Line 2085: nit: ..it.. http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2085 PS2, Line 2085: Returns true if reloadPartitition() succeeds, false :* otherwise. Instead of referring to specific methods, rephrase it to something like returns true if the reload of the partition is successful, false otherwise http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2089 PS2, Line 2089: refresh nit:call "reload" to be consistent ? http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1102 PS2, Line 1102: // HMS adds partitions in a transactional way. We try to refresh all the : // partitions that were added to HMS. If any partition refresh fails, we throw : // MetastoreNotificationNeedsInvalidateException exception. We skip refresh of the : // partitions if the table is not present in the catalog. nit: Not super clear what this is. How are HMS transactional semantics related to what we are doing here? http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1118 PS2, Line 1118: ); nit: include the partition name too http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1125 PS2, Line 1125: ; include the partition name http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1295 PS2, Line 1295: + "event.", getFullyQualifiedTblName()); There seem to be a lot of common code between the partition event processors. Can we consolidate it by having some helpers that factor out the common code and exception handling? -- To view, visit http://gerrit.cloudera.org:8080/13111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I213401329f3965dd81055197792ccf8a05368af5 Gerrit-Change-Number: 13111 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Krishna Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 25 Apr 2019 04:43:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7973: Add support for fine grained updates at partition level.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13111 ) Change subject: IMPALA-7973: Add support for fine grained updates at partition level. .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2902/ : 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/13111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I213401329f3965dd81055197792ccf8a05368af5 Gerrit-Change-Number: 13111 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Krishna Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 25 Apr 2019 02:40:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7973: Add support for fine grained updates at partition level.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13111 ) Change subject: IMPALA-7973: Add support for fine grained updates at partition level. .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2901/ : 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/13111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I213401329f3965dd81055197792ccf8a05368af5 Gerrit-Change-Number: 13111 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Krishna Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 25 Apr 2019 02:25:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7973: Add support for fine grained updates at partition level.
Vihang Karajgaonkar has posted comments on this change. (
http://gerrit.cloudera.org:8080/13111 )
Change subject: IMPALA-7973: Add support for fine grained updates at partition
level.
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:
http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1106
PS2, Line 1106: addedPartitions_
Would be good if you can log a useful information about the event like number
of partitions, isReplace, tablename etc.
http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1107
PS2, Line 1107: Map partSpec = new HashMap<>();
:
List fsList =
: msTbl_.getPartitionKeys();
: List partVals = partition.getValues();
: Preconditions.checkNotNull(partVals);
: Preconditions.checkState(fsList.size() ==
partVals.size());
: for (int i = 0; i < fsList.size(); i++) {
: partSpec.put(fsList.get(i).getName(),
partVals.get(i));
: }
We can avoid one unnecessary conversion from Partition -> partSpecMap ->
TPartitionKeyValue if we change the signature of the refreshPartitionIfExists
to take List here.
http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1194
PS2, Line 1194: Preconditions.checkNotNull(partitionAfter_);
: Map partSpec = new HashMap<>();
: List
fsList =
: msTbl_.getPartitionKeys();
: List partVals = partitionAfter_.getValues();
: Preconditions.checkNotNull(partVals);
: Preconditions.checkState(fsList.size() ==
partVals.size());
: for (int i = 0; i < fsList.size(); i++) {
: partSpec.put(fsList.get(i).getName(), partVals.get(i));
: }
same suggestion as above. Also, instead of duplicating this logic, create a
util method in the base class (or a static method)
http://gerrit.cloudera.org:8080/#/c/13111/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1266
PS2, Line 1266: droppedPartitions_
add a null check as well
--
To view, visit http://gerrit.cloudera.org:8080/13111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I213401329f3965dd81055197792ccf8a05368af5
Gerrit-Change-Number: 13111
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Bharath Krishna
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 25 Apr 2019 02:14:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7973: Add support for fine grained updates at partition level.
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/13111 )
Change subject: IMPALA-7973: Add support for fine grained updates at partition
level.
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13111/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:
http://gerrit.cloudera.org:8080/#/c/13111/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1286
PS1, Line 1286: debugLog("Could not refresh partitions of table {}
after a drop_partition event"
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/13111
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I213401329f3965dd81055197792ccf8a05368af5
Gerrit-Change-Number: 13111
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Bharath Krishna
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 25 Apr 2019 01:39:10 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7973: Add support for fine grained updates at partition level.
Anurag Mantripragada has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13111 ) Change subject: IMPALA-7973: Add support for fine grained updates at partition level. .. IMPALA-7973: Add support for fine grained updates at partition level. This patch adds suport for fine grained updates for add/drop/alter partition events. Currently, partition events invalidate the table. This can be very expensive for large tables. Here, we refresh partitions in case of add/drop/alter partition events. HMS processes add/drop partitions in a transaction. We throw MetastoreNotificationNeedsInvalidateException if any of the partition refreshes fails. Testing: Modified pre-existing tests for partition events to insead test if partitions are added/dropped/altered when event processing is enabled. Change-Id: I213401329f3965dd81055197792ccf8a05368af5 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 3 files changed, 183 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/13111/2 -- To view, visit http://gerrit.cloudera.org:8080/13111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I213401329f3965dd81055197792ccf8a05368af5 Gerrit-Change-Number: 13111 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Krishna Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-7973: Add support for fine grained updates at partition level.
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13111 Change subject: IMPALA-7973: Add support for fine grained updates at partition level. .. IMPALA-7973: Add support for fine grained updates at partition level. This patch adds suport for fine grained updates for add/drop/alter partition events. Currently, partition events invalidate the table. This can be very expensive for large tables. Here, we refresh partitions in case of add/drop/alter partition events. HMS processes add/drop partitions in a transaction. We throw MetastoreNotificationNeedsInvalidateException if any of the partition refreshes fails. Testing: Modified pre-existing tests for partition events to insead test if partitions are added/dropped/altered when event processing is enabled. Change-Id: I213401329f3965dd81055197792ccf8a05368af5 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 3 files changed, 182 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/13111/1 -- To view, visit http://gerrit.cloudera.org:8080/13111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I213401329f3965dd81055197792ccf8a05368af5 Gerrit-Change-Number: 13111 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada
