Sai Hemanth Gantasala has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20022 )

Change subject: IMPALA-11535: Skip older events in the event processor based on 
the latestRefreshEventID
......................................................................


Patch Set 12:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/20022/11/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/20022/11/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2939
PS11, Line 2939: {} partiti
> nit: let's also log the number of reloaded partitions, e.g. partBuilderToPa
Agreed!!


http://gerrit.cloudera.org:8080/#/c/20022/11/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/20022/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3224
PS11, Line 3224:         testTblName);
               :     hiveExecutor.execute();
               :     alterTableAddParameter(testTblNam
> These are done in testInsertEvents(). I think we don't need them.
Ack


http://gerrit.cloudera.org:8080/#/c/20022/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3233
PS11, Line 3233:  String testUnpartTb
> typo: "Even" -> "Event"
Done


http://gerrit.cloudera.org:8080/#/c/20022/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3233
PS11, Line 3233: fina
> nit: long
Ack


http://gerrit.cloudera.org:8080/#/c/20022/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3235
PS11, Line 3235: testInsert
> nit: use assertEquals() to have better error message
Ack


http://gerrit.cloudera.org:8080/#/c/20022/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3246
PS11, Line 3246:     lastSyncEventIdBefore = testTbl.getLastRefreshEventId();
> Can we test that REFRESH will also update LastRefreshEventId? E.g. do some
The Java tests are not capable of making the event processor lagged. I'm using 
the Java tests only to check if lastRefreshEventId is updated or not. So, this 
tests are not really capable of checking skipped events. I have added 
end-to-end to simulate the same.
P.S: I added a sanity test for this ask.


http://gerrit.cloudera.org:8080/#/c/20022/11/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/20022/11/tests/custom_cluster/test_events_custom_configs.py@324
PS11, Line 324:       "create table {}.{} (i int) partitioned by (year int) "
> Can we also test transactional tables?
Ack


http://gerrit.cloudera.org:8080/#/c/20022/11/tests/custom_cluster/test_events_custom_configs.py@345
PS11, Line 345:     self.run_stmt_in_hive(
              :       "insert into {db}.{tbl} partition (year=2024) values 
(10),(20),(30)"
              :       .format(db=unique_database, tbl=test_old_table_1))
              :     self.run_stmt_in_hive(
              :       "insert into {db}.{tbl} partition (year=2024) values 
(10),(20),(30)"
              :       .format(db=unique_database, tbl=test_old_table_1))
              :     self.run_stmt_in_hive(
              :       "insert into {db}.{tbl} partition (year=2024) values 
(10),(20),(30)"
              :       .format(db=unique_database, tbl=test_old_table_1))
              :     self.client.execute(
              :       "refresh {}.{}".format(unique_database, test_old_table_1))
> Could you double check these can finish in 5s? If not, some events might be
The test is running fine. I ran the test 10 times consecutively and it passed 
all the times.


http://gerrit.cloudera.org:8080/#/c/20022/11/tests/custom_cluster/test_events_custom_configs.py@363
PS11, Line 363:
> flake8: E122 continuation line missing indentation or outdented
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0dc5c7396d80616680d8a5805ce80db293b72e1
Gerrit-Change-Number: 20022
Gerrit-PatchSet: 12
Gerrit-Owner: Sai Hemanth Gantasala <saihema...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Aug 2023 04:55:53 +0000
Gerrit-HasComments: Yes

Reply via email to