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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
