Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-4029: Reduce memory requirements for storing file metadata ......................................................................
Patch Set 4: (10 comments) http://gerrit.cloudera.org:8080/#/c/6406/3/common/fbs/CatalogObjects.fbs File common/fbs/CatalogObjects.fbs: Line 40: // Total length of the block > It can be computed based on the offset, file length, and block size. No nee Good point. Left a TODO. Line 65: last_modification_time: long (id: 3); > I was mostly wondering if the field order matters. For example, could we sa Interesting thought. Left a TODO to investigate later. http://gerrit.cloudera.org:8080/#/c/6406/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java: Line 199: private static int[] loadBlockMetadata(FlatBufferBuilder fbb, > createBlockMetadata()? Done Line 303: private final boolean isCached_; > hasCachedReplica_ Done Line 372: private static short[] loadDiskIds(BlockLocation location, FileDescStats stats) { > mapDiskIds() or createDiskIds()? Done Line 380: return Shorts.toArray(Collections.<Short>emptyList()); > why not: hahah. I wonder if I could add another unnecessary step :P Line 387: List<Short> diskIDs = new ArrayList<Short>(storageIds.length); > Why not a short[]? The size is fixed and you can even use the 'i' index in Done Line 403: public boolean isCached() { return isCached_; } > hasCachedReplica() Done http://gerrit.cloudera.org:8080/#/c/6406/4/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: Line 110: return storedInImpaladCatalogCache_ || RuntimeEnv.INSTANCE.isTestEnv(); > Comment on the test env condition Done Line 287: // Indicate that this table is stored in an Impalad catalog cache. > no need for comment Done -- 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: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
