impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4789: Fix slow metadata loading due to inconsistent paths.
Date Fri, 27 Jan 2017 19:22:56 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-4789: Fix slow metadata loading due to inconsistent paths.
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5743/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

Line 275:         // a descendant of dirPath.
> do we already have test cases that will take this path?
This is the 'default' path for loading the metadata of a partitioned table, so almost all
tables load through this path. The above path is covered by things like INSERT, ALTER TABLE
ADD PARTITION, etc. so in terms of code coverage we have everything.


Line 693:     tblLocation = tblLocation.makeQualified(fs.getUri(), tblLocation);
> here and elsewhere: should we use createFullyQualifiedPath() to avoid the b
Yes. Didn't know about that. Done.


http://gerrit.cloudera.org:8080/#/c/5743/3/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java:

PS3, Line 440: !=
> nit: I feel an XOR makes the intention more clear than using a !=, especial
Rewrote this with a helper function to simplify.


PS3, Line 442: pUri.getScheme().equals(parentUri.getScheme())
> An example that could hit NPE. isDescendantPath(new Path("/foo/bar"), new P
You are right. Rewrote this with a helper function to simplify.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c881b7cb155032b82fba0e29350ca31de388d55
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message