Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11341 )
Change subject: IMPALA-7424: Reduce in-memory footprint of incremental stats ...................................................................... Patch Set 6: (13 comments) http://gerrit.cloudera.org:8080/#/c/11341/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11341/5//COMMIT_MSG@14 PS5, Line 14: deflate > gzip is not present in this change, so either state 'deflate-compressed' or Done http://gerrit.cloudera.org:8080/#/c/11341/5/common/thrift/CatalogService.thrift File common/thrift/CatalogService.thrift: http://gerrit.cloudera.org:8080/#/c/11341/5/common/thrift/CatalogService.thrift@302 PS5, Line 302: // Set if 'want_partition_stats' was set in TTableInfoSelector. Not set if the > and the partition has associated stats. just to disambiguate the case where Clarified. Fixed a related bug in FeCatalogUtils http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java File fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java: http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java@160 PS5, Line 160: titionSt > I think you're using this term since the prev. method was called getFiltere Done http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java: http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@524 PS5, Line 524: > and when its updated (from CatalogOpExecutor, through the Util to serialize Added a reference to setPartitionStatsBytes(). Didn't want to make it complex by adding all the callpaths. Interested ones can track it back using setPartitionStatsBytes()'s callers. http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java File fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java: http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java@76 PS5, Line 76: - > nit: add the hyphen Done http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java@109 PS5, Line 109: HdfsP > move this to the method javadoc. also mention the deletion on L122 Done http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java@123 PS5, Line 123: String debugString = > this was somewhat surprising at first... was wondering why the deletion if Its a no-op. Added more checks for this invariant as discussed. http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java@128 PS5, Line 128: > this is redundant with the prefix in L126 Done http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java@129 PS5, Line 129: > same here Done http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@819 PS5, Line 819: pri > Will remove in next PS Done http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/util/CompressionUtil.java File fe/src/main/java/org/apache/impala/util/CompressionUtil.java: http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/util/CompressionUtil.java@40 PS5, Line 40: if (input == null) return null; > precondition that input is not-null or return null if that's the case. same Done http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/main/java/org/apache/impala/util/CompressionUtil.java@59 PS5, Line 59: try{ > make these more symmetric. the other one returns in the try-block, this one Done http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/test/java/org/apache/impala/util/CompressionUtilTest.java File fe/src/test/java/org/apache/impala/util/CompressionUtilTest.java: http://gerrit.cloudera.org:8080/#/c/11341/5/fe/src/test/java/org/apache/impala/util/CompressionUtilTest.java@32 PS5, Line 32: si > Will update in next PS Done -- To view, visit http://gerrit.cloudera.org:8080/11341 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39f02ebfa0c6e9b0baedd0d76058a1b34efb5a02 Gerrit-Change-Number: 11341 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 31 Aug 2018 03:15:51 +0000 Gerrit-HasComments: Yes
