Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-5431: Remove redundant path exists checks during table load ......................................................................
Patch Set 4: (7 comments) As discussed, I'm checking the following, - Effect on CTAS when the path doesn't exist. - Running metadata benchmarks to see if there is some perf gain. Meanwhile, posting the rebased patch that addresses the comments. http://gerrit.cloudera.org:8080/#/c/7095/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: Line 267: // No need to load blocks for empty partitions list. > Right, but saving an RPC seems worth it. Done (in PS4). Line 725: } > remove while you are here Done http://gerrit.cloudera.org:8080/#/c/7095/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: Line 266: FileSystem fs = dirPath.getFileSystem(CONF); > Move this closer to where it is used. getFileSystem() tends to be expensive Done Line 305: if (fileStatusIter == null) { > I think you can make this a single line and remove the comment. Done Line 371: if (fileStatusIter == null) { > I think you can make this a single line and remove the comment. Done Line 1709: if (statuses == null) { > I think you can make this a single line and remove the comment. Done http://gerrit.cloudera.org:8080/#/c/7095/4/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java: Line 491: * the path does not exist. This helps simplifies the caller code in cases where > This helps simplify the caller code... Ouch, done. -- To view, visit http://gerrit.cloudera.org:8080/7095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id10ecf64ea2eda2d0f9299c0aa371933eca22281 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-HasComments: Yes
