impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4029: Reduce memory requirements for storing file metadata
Date Sat, 06 May 2017 20:09:52 GMT
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4029: Reduce memory requirements for storing file metadata
......................................................................


Patch Set 5:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/6406/5/fe/src/main/java/org/apache/impala/catalog/DiskIdMapper.java
File fe/src/main/java/org/apache/impala/catalog/DiskIdMapper.java:

Line 46:     private ConcurrentHashMap<String, Short> storageUuidToDiskId =
> add trailing _ to member vars
?


http://gerrit.cloudera.org:8080/#/c/6406/5/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

Line 161:      * Synthesizes the block metadata of a file represented by 'fileStatus' that
resides
> Done
i meant: how does it "synthesize" it? e.g., by creating artificial blocks on some boundary.


http://gerrit.cloudera.org:8080/#/c/6406/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

Line 112:         ListMap<TNetworkAddress> hostIndex, FileDescStats stats) throws IOException
{
numUnknown...?


Line 275:     }
provide more information (such as the fact that it's used in conjunction with an FbFileBlock...)


Line 283:       String[] ip_port = location.split(":");
"Constructs an FbFileBlock..."?


Line 291: 
numUnknown...?


Line 305:     // Use ~REPLICA_HOST_IDX_MASK to extract the cache info (stored in MSB).
doesn't guava have some kind of arrayutils.contains(loc.getcachedhosts(), loc.gethosts()[i])?


Line 327: 
"in the underlying...starts": fine to leave out, that's implied by fb semantics.


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

Line 287:         // Find the partition that this file belongs (if any).
numUnknown...?

does Reference<long> also work?


Line 741:       HashMap<Path, List<HdfsPartition>> partsByPath) {
is this from a rebase?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I483d3cadc9d459f71a310c35a130d073597b0983
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message