Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21031 )
Change subject: IMPALA-12709: Add support for hierarchical metastore event processing ...................................................................... Patch Set 34: (3 comments) http://gerrit.cloudera.org:8080/#/c/21031/34//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21031/34//COMMIT_MSG@63 PS34, Line 63: Following new events are added: > nit: It will be nice to have a shared interface for derived event kind so w Oh, please add Java unit test to assert behavior of these new event type individually. http://gerrit.cloudera.org:8080/#/c/21031/34/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/21031/34/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java@659 PS34, Line 659: CDP Hive-3 nit: Apache Hive-3 http://gerrit.cloudera.org:8080/#/c/21031/34/fe/src/main/java/org/apache/impala/catalog/events/RenameTableBarrierEvent.java File fe/src/main/java/org/apache/impala/catalog/events/RenameTableBarrierEvent.java: http://gerrit.cloudera.org:8080/#/c/21031/34/fe/src/main/java/org/apache/impala/catalog/events/RenameTableBarrierEvent.java@66 PS34, Line 66: // Pseudo-events processing sequence : private final Queue<MetastoreTableEvent> eventSequence_; : : // Event processing status of all the pseudo-events : private final Queue<Boolean> status_; I think using 2 Queue object per RenameTableBarrierEvent is overkill here. Perhaps a single Object to store all state + object level lock is sufficient and easier to test. For example: class RenameState { public synchronized mustDrop(); public synchronized doneDrop(boolean isSuccessful); public synchronized mustCreate(); public synchronized doneCreate(boolean isSuccessful); public synchronized isAllDone(); public synchronized isAllSuccessful(); public synchronized isAllFailed(); } -- To view, visit http://gerrit.cloudera.org:8080/21031 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76d8a739f9db6d40f01028bfd786a85d83f9e5d6 Gerrit-Change-Number: 21031 Gerrit-PatchSet: 34 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Tue, 21 Jan 2025 23:10:58 +0000 Gerrit-HasComments: Yes
