[email protected] has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21065 )

Change subject: IMPALA-12832: Implicit invalidate metadata on event failures
......................................................................


Patch Set 21:

(8 comments)

Rework of patchset-21 comments

http://gerrit.cloudera.org:8080/#/c/21065/21/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/21065/21/be/src/catalog/catalog-server.cc@190
PS21, Line 190: Set this value to ~0.0 to fail the EP consistently
> This seems stale.
Done


http://gerrit.cloudera.org:8080/#/c/21065/21/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/21065/21/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@887
PS21, Line 887: warnLog
> This should be logged at error level.
Done. Changed to error log at line 895 as well.


http://gerrit.cloudera.org:8080/#/c/21065/16/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/21065/16/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@677
PS16, Line 677:
> +1 for Quanlong's comment, < looks safer than <=
Done


http://gerrit.cloudera.org:8080/#/c/21065/21/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/21065/21/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@685
PS21, Line 685: Event process failed
> Could be more descriptive, e.g. "Event processing failed due to error injec
Done. Have added a private method in MetastoreEvent


http://gerrit.cloudera.org:8080/#/c/21065/21/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1298
PS21, Line 1298: warnLog
> This should be logged at error level.
Done


http://gerrit.cloudera.org:8080/#/c/21065/21/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1802
PS21, Line 1802: warnLog
> This should be logged at error level.
Done


http://gerrit.cloudera.org:8080/#/c/21065/21/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@3105
PS21, Line 3105: warnLog
> This should be logged at error level.
Done. Changed at line 3113 as well.


http://gerrit.cloudera.org:8080/#/c/21065/21/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File 
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:

http://gerrit.cloudera.org:8080/#/c/21065/21/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1189
PS21, Line 1189:                 event.warnLog("Unable to handle event 
processing failure");
> Probably this should be logged at error level?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c
Gerrit-Change-Number: 21065
Gerrit-PatchSet: 21
Gerrit-Owner: 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: Sai Hemanth Gantasala <[email protected]>
Gerrit-Comment-Date: Thu, 07 Mar 2024 12:41:42 +0000
Gerrit-HasComments: Yes

Reply via email to