Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8851 )
Change subject: IMPALA-3193: Show table's comment on show tables ...................................................................... Patch Set 15: (2 comments) http://gerrit.cloudera.org:8080/#/c/8851/15/fe/src/main/java/org/apache/impala/service/MetadataOp.java File fe/src/main/java/org/apache/impala/service/MetadataOp.java: http://gerrit.cloudera.org:8080/#/c/8851/15/fe/src/main/java/org/apache/impala/service/MetadataOp.java@279 PS15, Line 279: for (Table table: fe.getTables(db.getName(), tablePatternMatcher, user)) { : String tabName = table.getName(); > Are we certain that this code doesn't introduce the same infinite wait for I confirmed the problem should be solved with my change. The cause of the problem is JUnit test requires tables loading explicitly if they are not loaded. This is the reason why I introduce a new code at ImpaladTestCatalog.java. http://gerrit.cloudera.org:8080/#/c/8851/15/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java File fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java: http://gerrit.cloudera.org:8080/#/c/8851/15/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java@107 PS15, Line 107: /** : * Overrides ImpaladCatalog.getTables to load the table metadata if it is missing. : */ : @Override : public List<Table> getTables(String dbName, PatternMatcher matcher) : throws DatabaseNotFoundException { : List<Table> tables = super.getTables(dbName, matcher); : : // The table was not yet loaded. Load it in to the catalog and try getTable() : // again. : for (int i = 0; i < tables.size(); ++i) { : Table table = tables.get(i); : // Table doesn't exist or is already loaded. Just return it. : if (table != null && !table.isLoaded()) { : try { : tables.set(i, getLoadedTable(dbName, table.getName())); : } catch (CatalogException e) { : LOG.error(String.format("Error loading table: %s.%s", dbName, table.getName()), : e); : } : } : } : return tables; : } > Where is the ImpaladTestCatalog::getTables() called? During JUnit test, it should be called at "impaladCatalog_.get().getTables(dbName, matcher)". See https://gerrit.cloudera.org/#/c/8851/15/fe/src/main/java/org/apache/impala/service/Frontend.java at 625. -- To view, visit http://gerrit.cloudera.org:8080/8851 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I44f814af05db6f3c027718ade9f474f8b8153bcd Gerrit-Change-Number: 8851 Gerrit-PatchSet: 15 Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com> Gerrit-Comment-Date: Mon, 12 Feb 2018 23:17:02 +0000 Gerrit-HasComments: Yes