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

Change subject: IMPALA-14582: Deflake test_show_create_table_with_stats
......................................................................


Patch Set 2: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/23728/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23728/2//COMMIT_MSG@9
PS2, Line 9: is flaky can be flaky
nit: "is flaky"


http://gerrit.cloudera.org:8080/#/c/23728/2//COMMIT_MSG@10
PS2, Line 10: between ALTER TABLE queries that it replays
nit: "is not handled/retried correctly in coordinator side"


http://gerrit.cloudera.org:8080/#/c/23728/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:

http://gerrit.cloudera.org:8080/#/c/23728/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@613
PS2, Line 613:         String.format("load table data %s.%s", tname.db_name, 
tname.table_name));
nit: "show create table %s.%s"


http://gerrit.cloudera.org:8080/#/c/23728/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@612
PS2, Line 612:     Frontend.RetryTracker retries = new Frontend.RetryTracker(
             :         String.format("load table data %s.%s", tname.db_name, 
tname.table_name));
             :     while (true) {
             :       try {
             :         if (withStats) {
             :           // Get show_create_table_partition_limit from request, 
default to 1000 if not
             :           // set
             :           int partitionLimit = 
req.isSetShow_create_table_partition_limit() ?
             :               req.getShow_create_table_partition_limit() :
             :               1000;
             :           return ToSqlUtils.getCreateTableWithStatsSql(table, 
partitionLimit);
             :         } else {
             :           return ToSqlUtils.getCreateTableSql(table);
             :         }
             :       } catch (InconsistentMetadataFetchException e) { 
retries.handleRetryOrThrow(e); }
             :     }
nit: It'd be more consistent to extract these into a method in Frontend.java. 
We handle the retry there, e.g. getMetadataTableNames().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I397b9502d92bfd756929be8e851661fd9246dd5e
Gerrit-Change-Number: 23728
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Tue, 02 Dec 2025 03:30:17 +0000
Gerrit-HasComments: Yes

Reply via email to