Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9693 )
Change subject: IMPALA-5842: Write page index in Parquet files ...................................................................... Patch Set 15: (5 comments) Thanks for your patience. I'm read to +2 after a couple of tweaks to the MemTracker stuff. http://gerrit.cloudera.org:8080/#/c/9693/15/be/src/exec/hdfs-parquet-table-writer.cc File be/src/exec/hdfs-parquet-table-writer.cc: http://gerrit.cloudera.org:8080/#/c/9693/15/be/src/exec/hdfs-parquet-table-writer.cc@109 PS15, Line 109: HdfsSinkTable HdfsTableSink? http://gerrit.cloudera.org:8080/#/c/9693/15/be/src/exec/hdfs-parquet-table-writer.cc@118 PS15, Line 118: table_sink_mem_tracker_->Release(page_index_memory_consumption_); Can we do this in Close() instead? We have generally been trying to do resource releasing in Close() so that the sequencing of resource release is more explicit (goes against RAII but there are pros and cons of that). http://gerrit.cloudera.org:8080/#/c/9693/15/be/src/exec/hdfs-parquet-table-writer.cc@318 PS15, Line 318: MemTracker *table_sink_mem_tracker_; nit: MemTracker* http://gerrit.cloudera.org:8080/#/c/9693/15/be/src/exec/hdfs-parquet-table-writer.cc@766 PS15, Line 766: Consume We prefer TryConsume() and returning a MEM_LIMIT_EXCEEDED error synchronously so that we don't depend on the memory limit being enforced asynchronously. HdfsTableSink is a bad offender in terms of using Consume(), which we need to fix at some point. http://gerrit.cloudera.org:8080/#/c/9693/13/testdata/bin/load-dependent-tables.sql File testdata/bin/load-dependent-tables.sql: http://gerrit.cloudera.org:8080/#/c/9693/13/testdata/bin/load-dependent-tables.sql@77 PS13, Line 77: (cs CHAR(5), cl CHAR(140), vc VARCHAR(32)) > My bad, I thought if I run a complete data load after formatting, it will e I ended up running these statements manually via copy-and-paste into beeline. I think this should run during pre-commit if we do a full data load. -- To view, visit http://gerrit.cloudera.org:8080/9693 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9 Gerrit-Change-Number: 9693 Gerrit-PatchSet: 15 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Anonymous Coward #248 Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 08 May 2018 22:14:41 +0000 Gerrit-HasComments: Yes
