Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17848 )
Change subject: IMPALA-9857: Batching of consecutive partition events ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/17848/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java: http://gerrit.cloudera.org:8080/#/c/17848/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@878 PS2, Line 878: * when isInsertEventContext is false, it's version number to remove > line too long (91 > 90) Done http://gerrit.cloudera.org:8080/#/c/17848/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/17848/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1277 PS2, Line 1277: Preconditions.checkArgument(MetastoreEventType.CREATE_DATABASE.equals(getEventType())); > line too long (93 > 90) Done http://gerrit.cloudera.org:8080/#/c/17848/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1336 PS2, Line 1336: Preconditions.checkArgument(MetastoreEventType.ALTER_DATABASE.equals(getEventType())); > line too long (92 > 90) Done http://gerrit.cloudera.org:8080/#/c/17848/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1392 PS2, Line 1392: Preconditions.checkArgument(MetastoreEventType.DROP_DATABASE.equals(getEventType())); > line too long (91 > 90) Done http://gerrit.cloudera.org:8080/#/c/17848/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1641 PS2, Line 1641: return versionNumberFromEvent_ == versionNumberOfEvent && serviceIdFromEvent_.equals( > Would it not be more performant if we batch alter partition events for the Since the default value of polling is 1s and we fetch the events in the batch of 1000 from metastore, even with the existing patch a single stream of 1800 events is broken into several batches of few hundred events. There is a trade off here with the polling interval and the batch size. Higher the polling interval, bigger the batch sizes but we increase the time it takes eventually to sync all the events because we are polling at a slower rate. Hence the performance may not improve significantly since the batch sizes are always in a few hundreds based on the experiment that I ran. http://gerrit.cloudera.org:8080/#/c/17848/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1728 PS2, Line 1728: this.baseEvent_ = baseEvent; > Should we assert here that baseEvent can only be either AlterPartitionEvent Wouldn't that be inconsistent with the use of generic types here. We only need to special-case for Insert since the self-event identifiers are different in case of insert. That is handled in the getSelfEventContext() method. In future we can probably batch ALTER_TABLE and ALTER_DATABASE events too using this same class with small modification to the code. http://gerrit.cloudera.org:8080/#/c/17848/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/17848/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2384 PS2, Line 2384: mockEvents.addAll(createMockEvents(startEventId + mockEvents.size(), 1, "ALTER_PARTITION", > line too long (94 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/17848 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d27a68a64436d31731e9a219b1efd6fc842de73 Gerrit-Change-Number: 17848 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Sourabh Goyal <soura...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Thu, 16 Sep 2021 18:36:23 +0000 Gerrit-HasComments: Yes
