Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201701271922.v0RJMuGr032061@ip-10-146-233-104.ec2.internal>
Date: Fri, 27 Jan 2017 19:22:56 +0000
From: "Alex Behm (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
CC: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>,
        Dan Hecht <dhecht@cloudera.com>,
        Marcel Kornacker <marcel@cloudera.com>,
        Bharath Vissapragada <bharathv@cloudera.com>
Reply-To: alex.behm@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4789=3A_Fix_slow_metadata_loading_due_to_inconsistent_paths=2E=0A?=
In-Reply-To: <gerrit.1484841809000.I8c881b7cb155032b82fba0e29350ca31de388d55@gerrit.cloudera.org>
References: <gerrit.1484841809000.I8c881b7cb155032b82fba0e29350ca31de388d55@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Fri, 27 Jan 2017 19:23:05 -0000

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