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

Reply via email to