Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10550 )
Change subject: IMPALA-7078: Part 1: improve memory consumption of wide Avro scans ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc File be/src/exec/hdfs-avro-scanner.cc: http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc@498 PS7, Line 498: starts off with AtCapacity() == true. > under which case do we get here with row_batch already AtCapacity()? Not as We can get there if the tuple buffer is > 8mb, e.g. for extremely wide rows. http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/exec/hdfs-avro-scanner.cc@498 PS7, Line 498: mke > make Done http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.h File be/src/runtime/mem-tracker.h: http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.h@238 PS7, Line 238: Must not be used if > do we have cases that transfer in this case? if so, what's the recommended I think the data stream receiver might have this case. For now we do a Release() and Consume(). Maybe, ideally, we should do a TryTransfer() or something like that? http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.cc File be/src/runtime/mem-tracker.cc: http://gerrit.cloudera.org:8080/#/c/10550/7/be/src/runtime/mem-tracker.cc@236 PS7, Line 236: && all_trackers_[ancestor_idx - 1] == dst->all_trackers_[dst_ancestor_idx - 1]) { > should we dcheck that the limits are not set? These MemTrackers can have limits, it's the ones below the common ancestors that are not allowed to have limits. ConsumeLocal() enforces this. -- To view, visit http://gerrit.cloudera.org:8080/10550 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iebd2600b4784fd19696c9b92eefb7d7e9db0c80b Gerrit-Change-Number: 10550 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 06 Jun 2018 00:26:22 +0000 Gerrit-HasComments: Yes
