Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/8235 )
Change subject: IMPALA-5429: Multi threaded block metadata loading ...................................................................... Patch Set 7: (9 comments) http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@221 PS7, Line 221: // are excluded because they are considered hidden from Impala's perspecitve > Shrink: Done http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@226 PS7, Line 226: // Number of files skipped while computeing the block metadata information. > Shrink: Done http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@248 PS7, Line 248: // If set to true, reloads the block metadata only when the files in this path > reloads the file metadata (let's try to use the new "file metadata" termino Done. Changed at other places too. http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@249 PS7, Line 249: // has changed since last load (more details in hasFileChanged()). > have changed Done http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@785 PS7, Line 785: * much higher throughput of RPC calls for listStatus/listFiles. Based on our > ... RPC calls for listStatus/listFiles. For simplicity, the filesystem type Done http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@791 PS7, Line 791: * This method is not optimized for tables with mixed partition types (partitions mapped > I don'd think we need this much detail, how about folding a simple sentence Done http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@794 PS7, Line 794: * This may not work well with the mixed partition types and needs to fixed. > Don't think this is needed. Done http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@819 PS7, Line 819: // For tables without partitions, we have no block metadata to load. > remove "," Done http://gerrit.cloudera.org:8080/#/c/8235/7/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@841 PS7, Line 841: LOG.error("Encountered an error loading block metadata for table: " + > Could this flood the log when HDFS is under pressure or there is an intermi Very good point. This can likely flood the log. IMO we should have some stack traces of errors for supportability. Added some code to limit the number of errors to log (100) and logged the total failed tasks. Please let me know if you prefer some other approach. -- To view, visit http://gerrit.cloudera.org:8080/8235 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I07eaa7151dfc4d56da8db8c2654bd65d8f808481 Gerrit-Change-Number: 8235 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Mostafa Mokhtar <mmokh...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Sun, 22 Oct 2017 20:05:45 +0000 Gerrit-HasComments: Yes