Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20131 )

Change subject: IMPALA-12152: Add query options to wait for HMS events sync up
......................................................................


Patch Set 33:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20131/31/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/20131/31/fe/src/main/java/org/apache/impala/service/Frontend.java@2294
PS31, Line 2294: queryCtx.setWarnings(errorMsg);
> I think existing query_handle->GetAnalysisWarnings() should be the preferre
I got this a try and realised that another advantage of using 
coord->GetErrorLog() is that the message will appear in the "Erros:" section of 
the profile:
https://github.com/apache/impala/blob/2e59bbae37dfd0b4791aaed775d6de9406ad34d8/be/src/service/impala-server.cc#L1574-L1575
Maybe we should find another place in profile to show this message.


http://gerrit.cloudera.org:8080/#/c/20131/31/tests/metadata/test_event_processing_base.py
File tests/metadata/test_event_processing_base.py:

http://gerrit.cloudera.org:8080/#/c/20131/31/tests/metadata/test_event_processing_base.py@144
PS31, Line 144:   def _run_event_based_replication_tests_impl(cls, 
filesystem_client, transactional=True):
              :     """Hive Replication relies on the insert events generated
> If you are referring to default impala clients inside ImpalaTestSuite, they
Yeah, at least we can do that for tests that are executed serially. Not sure if 
e2e tests run in parallel share the clients.


http://gerrit.cloudera.org:8080/#/c/20131/32/tests/metadata/test_event_processing_base.py
File tests/metadata/test_event_processing_base.py:

http://gerrit.cloudera.org:8080/#/c/20131/32/tests/metadata/test_event_processing_base.py@37
PS32, Line 37:     with cls.create_impala_client() as impala_client:
             :       #
> nit: You can use with-as instead of try-catch
Cool, done.


http://gerrit.cloudera.org:8080/#/c/20131/32/tests/metadata/test_event_processing_base.py@60
PS32, Line 60:
> nit: Please address this.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36ac941bb2c2217b09fcfa2eb567b011b38efa2a
Gerrit-Change-Number: 20131
Gerrit-PatchSet: 33
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Fri, 24 Jan 2025 11:53:11 +0000
Gerrit-HasComments: Yes

Reply via email to