Re: Why not use shared_ptr or other ref counting to handle RowBatch's auxilliary memory?

2016-08-31 Thread Tim Armstrong
There are various reasons - mainly we want more control over memory usage and accounting than shared_ptr allows. Generally we avoid shared_ptr in Impala since it makes it harder to reason about when resources are released. E.g. we typically want to know/control exactly when memory is freed up.

[Impala-ASF-CR] IMPALA-4049: fix empty batch handling NLJ build side

2016-08-31 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4049: fix empty batch handling NLJ build side .. Patch Set 2: (1 comment) It seems like it would be very difficult to trigger without a subplan

[Impala-ASF-CR] IMPALA-4049: fix empty batch handling NLJ build side

2016-08-31 Thread Tim Armstrong (Code Review)
-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>

[Impala-ASF-CR] IMPALA-3308: Get expr-test passing on PPC64LE

2016-08-31 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3308: Get expr-test passing on PPC64LE .. Patch Set 1: * from what I can see only the ExprValue() and ExprValue(std::string&) constructors are

[Impala-ASF-CR] IMPALA-3308: Get expr-test passing on PPC64LE

2016-08-31 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3308: Get expr-test passing on PPC64LE .. Patch Set 1: Actually I think we should go further to prevent such bugs in future. Currently it's still

[Impala-ASF-CR] IMPALA-3308: Get expr-test passing on PPC64LE

2016-08-31 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3308: Get expr-test passing on PPC64LE .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4186/1/be/src/exprs/literal.cc File be/src/exprs

[Impala-ASF-CR] IMPALA-4051: Avoid unnecessary copy of RowDescriptor into RowBatch

2016-08-31 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4051: Avoid unnecessary copy of RowDescriptor into RowBatch .. Patch Set 1: Created IMPALA-4051. It sometimes feels like overkill to create JIRAs

Re: Documentation on how to symbolize Minidumps

2016-08-31 Thread Tim Armstrong
Looks good to me - thank you for putting this together. I see you've already identified minidump2core as a todo - that would also be helpful since sometimes that lets you inspect local variables and arguments, which I don't think you can do with the stackwalk tool. On Wed, Aug 31, 2016 at 3:42

[Impala-ASF-CR] IMPALA-4051: Avoid unnecessary copy of RowDescriptor into RowBatch

2016-08-31 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2). Change subject: IMPALA-4051: Avoid unnecessary copy of RowDescriptor into RowBatch .. IMPALA-4051: Avoid unnecessary copy of RowDescriptor into RowBatch This commit changes

[Impala-ASF-CR] IMPALA-4049: fix empty batch handling NLJ build side

2016-08-31 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2). Change subject: IMPALA-4049: fix empty batch handling NLJ build side .. IMPALA-4049: fix empty batch handling NLJ build side Memory from the build side of a nested loop join

[Impala-ASF-CR] IMPALA-4049: fix empty batch handling NLJ build side

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4182 Change subject: IMPALA-4049: fix empty batch handling NLJ build side .. IMPALA-4049: fix empty batch handling NLJ build side Memory

[Impala-ASF-CR] Avoid unnecessary copy of RowDescriptor into RowBatch

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4181 Change subject: Avoid unnecessary copy of RowDescriptor into RowBatch .. Avoid unnecessary copy of RowDescriptor into RowBatch

[Impala-ASF-CR] Revert "Use impala-python when building shell tarball"

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Revert "Use impala-python when building shell tarball" .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4176 To u

[Impala-ASF-CR] IMPALA-4037,IMPALA-4038: fix locking during query cancellation

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#4). Change subject: IMPALA-4037,IMPALA-4038: fix locking during query cancellation .. IMPALA-4037,IMPALA-4038: fix locking during query cancellation * Refactor the child query

[Impala-ASF-CR] IMPALA-2831: Bound the number of scanner threads per scan node.

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-2831: Bound the number of scanner threads per scan node. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4174/1/be/src/exec/hdfs-scan

[Impala-ASF-CR] IMPALA-2831: Bound the number of scanner threads per scan node.

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-2831: Bound the number of scanner threads per scan node. .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4174/1/be/src/exec/hdfs-scan

[Impala-ASF-CR] IMPALA-4019: initialize member variables in HdfsTableSink

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4019: initialize member variables in HdfsTableSink .. Patch Set 1: -Code-Review -- To view, visit http://gerrit.cloudera.org:8080/4171 To unsubscribe

[Impala-ASF-CR] IMPALA-4019: initialize member variables in HdfsTableSink

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4019: initialize member variables in HdfsTableSink .. Patch Set 1: Code-Review-2 I started a GVO. Marking -2 so it won't accidentally get merged

[Impala-ASF-CR] IMPALA-4019: initialize member variables in HdfsTableSink

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4171 Change subject: IMPALA-4019: initialize member variables in HdfsTableSink .. IMPALA-4019: initialize member variables in HdfsTableSink

[Impala-ASF-CR] IMPALA-3905: Add single-threaded scan node.

2016-08-30 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3905: Add single-threaded scan node. .. Patch Set 3: (10 comments) http://gerrit.cloudera.org:8080/#/c/4113/3/be/src/exec/hdfs-avro-scanner.cc File

[Impala-ASF-CR] IMPALA-3808: Add incubating DISCLAIMER from the Incubator Branding Guide

2016-08-29 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3808: Add incubating DISCLAIMER from the Incubator Branding Guide .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4160

[Impala-ASF-CR] IMPALA-4027:Memory leak with ExprCtxs not free

2016-08-29 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4027:Memory leak with ExprCtxs not free .. Patch Set 2: Code-Review+2 Rebased, carry +2 -- To view, visit http://gerrit.cloudera.org:8080/4132

[Impala-ASF-CR] IMPALA-3671: Add query option to limit scratch space usage

2016-08-26 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3671: Add query option to limit scratch space usage .. Patch Set 5: (20 comments) http://gerrit.cloudera.org:8080/#/c/3938/4/be/src/runtime/buffered

[Impala-ASF-CR] IMPALA-3671: Add query option to limit scratch space usage

2016-08-26 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#5). Change subject: IMPALA-3671: Add query option to limit scratch space usage .. IMPALA-3671: Add query option to limit scratch space usage Currently we can only disable spilling

[Impala-ASF-CR] IMPALA-4027:Memory leak with ExprCtxs not free

2016-08-26 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4027:Memory leak with ExprCtxs not free .. Patch Set 1: Code-Review+1 The change looks good to me, nice catch. Have you submitted a license agreement

[Impala-ASF-CR] IMPALA-3201: reservation implementation for new buffer pool

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3201: reservation implementation for new buffer pool .. Patch Set 7: (1 comment) Added reservation counters and reporting, plus simplified the linkage

[Impala-ASF-CR] IMPALA-3201: reservation implementation for new buffer pool

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#8). Change subject: IMPALA-3201: reservation implementation for new buffer pool .. IMPALA-3201: reservation implementation for new buffer pool This patch implements the reservation

[Impala-ASF-CR] IMPALA-3201: reservation implementation for new buffer pool

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#7). Change subject: IMPALA-3201: reservation implementation for new buffer pool .. IMPALA-3201: reservation implementation for new buffer pool This patch implements the reservation

Dictionary decoding with SIMD instructions

2016-08-25 Thread Tim Armstrong
A blog post related to some work going on in the parquet-cpp project to speed up dictionary decoding. It's still a very rough prototype. Might be worth keeping an eye on. http://lemire.me/blog/2016/08/25/faster-dictionary-decoding-with-simd-instructions/

[Impala-ASF-CR] IMPALA-3905: Add single-threaded scan node.

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3905: Add single-threaded scan node. .. Patch Set 1: (6 comments) A few more comments. I think I understand the core of this a bit better - will wait

[Impala-ASF-CR] System Database (preview for frontend)

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: System Database (preview for frontend) .. Patch Set 13: (17 comments) I took another look over this, looks like we're making progress. I know you have other

Re: [VOTE] Bylaw change to make branch creation or deletion lazy consensus

2016-08-25 Thread Tim Armstrong
+1 (Binding) On Thu, Aug 25, 2016 at 3:18 PM, Matthew Jacobs wrote: > +1 (binding) > > On Thu, Aug 25, 2016 at 3:01 PM, Marcel Kornacker > wrote: > > +1 (binding) > > > > On Thu, Aug 25, 2016 at 1:48 PM, Jim Apple wrote: > >> Oh,

[Impala-ASF-CR] IMPALA-4006: dangerous rm -rf statements in scripts

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4006: dangerous rm -rf statements in scripts .. Patch Set 6: We already have set -euo pipefail at the top of all of the scripts in the repo, which turns

[Impala-ASF-CR] IMPALA-1659: Netezza compatibility functions: metadata

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-1659: Netezza compatibility functions: metadata .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4063 To unsubscribe

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#10). Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder The main outcome of this patch is to split out

[Impala-ASF-CR] Stricter clang-format: set DerivePointerAlignment to false.

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Stricter clang-format: set DerivePointerAlignment to false. .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4127 To unsubscribe

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. Patch Set 9: (17 comments) http://gerrit.cloudera.org:8080/#/c/3873/9/be/src/exec/analytic-eval

[Impala-CR](cdh5-trunk) IMPALA-2809: Improve ByteSwap with builtin function or SSSE3 or AVX2.

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has abandoned this change. Change subject: IMPALA-2809: Improve ByteSwap with builtin function or SSSE3 or AVX2. .. Abandoned Moved to https://gerrit.cloudera.org/#/c/4124/1 -- To view, visit http

[Impala-ASF-CR] IMPALA-1659: Netezza compatibility functions: metadata

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-1659: Netezza compatibility functions: metadata .. Patch Set 4: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/4063/4/be/src/exprs/expr

[Impala-ASF-CR] IMPALA-3905: Add single-threaded scan node.

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3905: Add single-threaded scan node. .. Patch Set 1: (22 comments) I did an initial pass. I still need to think through some of the details but I

[Impala-ASF-CR] IMPALA-3979: Fix dynamic linking for Impala

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3979: Fix dynamic linking for Impala .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4108 To unsubscribe, visit http

[Impala-ASF-CR] IMPALA-3979: Fix dynamic linking for Impala

2016-08-25 Thread Tim Armstrong (Code Review)
Tim Armstrong has submitted this change and it was merged. Change subject: IMPALA-3979: Fix dynamic linking for Impala .. IMPALA-3979: Fix dynamic linking for Impala When building Impala, the user can choose between two

[Impala-ASF-CR] IMPALA-3979: Fix dynamic linking for Impala

2016-08-24 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3979: Fix dynamic linking for Impala .. Patch Set 3: I'll go ahead and merge this. I verified it builds on all supported OSes -- To view, visit http

[Impala-ASF-CR] IMPALA-3979: Fix dynamic linking for Impala

2016-08-24 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3979: Fix dynamic linking for Impala .. Patch Set 2: Code-Review+2 Carry +2 -- To view, visit http://gerrit.cloudera.org:8080/4108 To unsubscribe

[Impala-ASF-CR] Remove unused MemTracker debug logging

2016-08-24 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4117 Change subject: Remove unused MemTracker debug logging .. Remove unused MemTracker debug logging The MemTracker code included debug

Re: Error linking impalad with -build_shared_libs option

2016-08-24 Thread Tim Armstrong
The fix works for me too, thanks Martin! I gave the patch a +1 but will upgrade to +2 if it seems everyone is happy with it. Cheers, Tim On Wed, Aug 24, 2016 at 8:15 AM, Yonghyun Hwang wrote: > Hello Martin, > > Your fix works great! :) As for the build failure, what

[Impala-ASF-CR] Fix dynamic linking for Impala

2016-08-24 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Fix dynamic linking for Impala .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4108/1//COMMIT_MSG Commit Message: Line 7: Fix dynamic linking

[Impala-ASF-CR] Fix dynamic linking for Impala

2016-08-24 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Fix dynamic linking for Impala .. Patch Set 1: Code-Review+1 The fix works for me and this is cleaner. As far as I can tell the webserver was only a separate

[Impala-ASF-CR] IMPALA-3832: test invalid data handling in lzo text scanner

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3832: test invalid data handling in lzo text scanner .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4096/3/tests/query_test

[Impala-ASF-CR] IMPALA-3832: test invalid data handling in lzo text scanner

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3832: test invalid data handling in lzo text scanner .. Patch Set 4: Code-Review+2 Carry +2 -- To view, visit http://gerrit.cloudera.org:8080/4096

[Impala-ASF-CR] IMPALA-3832: test invalid data handling in lzo text scanner

2016-08-23 Thread Tim Armstrong (Code Review)
: Ib707014c1fcfb80cb8076f644fc2b62a5ae758d7 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>

[Impala-ASF-CR] IMPALA-3945: Forbid create text table with nonsensical delimiter combinations.

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3945: Forbid create text table with nonsensical delimiter combinations. .. Patch Set 3: Can we abandon this for now if we don't have any intent to merge

[Impala-CR](cdh5-trunk) IMPALA-2700: ASCII NUL characters are doubled on insert into text tables

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has abandoned this change. Change subject: IMPALA-2700: ASCII NUL characters are doubled on insert into text tables .. Abandoned This was already merged to master: https://gerrit.cloudera.org/#/c/3876

[Impala-ASF-CR] IMPALA-3832: test invalid data handling in lzo text scanner

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3832: test invalid data handling in lzo text scanner .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/4096/2/tests/query_test

[Impala-ASF-CR] IMPALA-3832: test invalid data handling in lzo text scanner

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#3). Change subject: IMPALA-3832: test invalid data handling in lzo text scanner .. IMPALA-3832: test invalid data handling in lzo text scanner This adds the lzo text scanner

[Impala-ASF-CR] Add .clang-format for Impala's C++ style

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Add .clang-format for Impala's C++ style .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/3886/6/.clang-format File .clang-format: Line 1

[Impala-ASF-CR] IMPALA-3832: test invalid data handling in lzo text scanner

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4096 Change subject: IMPALA-3832: test invalid data handling in lzo text scanner .. IMPALA-3832: test invalid data handling in lzo text

[Impala-ASF-CR] IMPALA-4006: dangerous rm -rf statements in scripts

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4006: dangerous rm -rf statements in scripts .. Patch Set 3: Code-Review+1 Will wait for Michael's +1 too. -- To view, visit http://gerrit.cloudera.org

[Impala-ASF-CR] IMPALA-1659: Netezza compatibility functions: metadata

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-1659: Netezza compatibility functions: metadata .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/4063/2/be/src/exprs/expr-test.cc File

[Impala-ASF-CR] Infomation schema (preview for frontend)

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Infomation_schema (preview for frontend) .. Patch Set 6: (18 comments) Did a pass over it and made some general comments. I think we should do some cleanup

[Impala-ASF-CR] Infomation schema (preview for frontend)

2016-08-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Infomation_schema (preview for frontend) .. Patch Set 6: Could you run clang-format on the backend to clean up the formatting? If you get .clang_format from

[Impala-ASF-CR] IMPALA-3201: reservation implementation for new buffer pool

2016-08-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3201: reservation implementation for new buffer pool .. Patch Set 6: Updated to use the UsedReservation/UnusedReservation terminology I discussed

[Impala-ASF-CR] IMPALA-3201: reservation implementation for new buffer pool

2016-08-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#6). Change subject: IMPALA-3201: reservation implementation for new buffer pool .. IMPALA-3201: reservation implementation for new buffer pool This patch implements the reservation

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#9). Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder The main outcome of this patch is to split out

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. Patch Set 8: Rebased this onto master MemLimitExceeded() changes. Had to make a few straightforward

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#8). Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder The main outcome of this patch is to split out

[Impala-ASF-CR] IMPALA-4006: impala-config.sh contains dangerous rm -rf statements

2016-08-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4006: impala-config.sh contains dangerous rm -rf statements .. Patch Set 2: (2 comments) Mostly looks good (aside from LOG_DIR that needs to be added

[Impala-ASF-CR] IMPALA-4006 impala-config.sh contains dangerous rm -rf statements

2016-08-22 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4006 impala-config.sh contains dangerous rm -rf statements .. Patch Set 1: (13 comments) This seems like a good practice, but I'm not clear on how you

[Impala-ASF-CR] IMPALA-3981: Fix crash when accessing statestored / catalogd /memz page

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3981: Fix crash when accessing statestored / catalogd /memz page .. Patch Set 10: Code-Review+2 +2 after rebase -- To view, visit http

[Impala-ASF-CR] IMPALA-(3895,3859): Don't log file data on parse errors

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-(3895,3859): Don't log file data on parse errors .. Patch Set 3: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4020/3/tests/common

[Impala-ASF-CR] IMPALA-3943: Adhere to abort on error in ProcessFooter().

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3943: Adhere to abort_on_error in ProcessFooter(). .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/3862/7/be/src/exec/hdfs-parquet

[Impala-ASF-CR] IMPALA-3662: Don't double allocate tuples buffer in parquet scanner

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3662: Don't double allocate tuples buffer in parquet scanner .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4064/1

[Impala-ASF-CR] IMPALA-3662: Don't double allocate tuples' buffer in parquet scanner

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3662: Don't double allocate tuples' buffer in parquet scanner .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4064/1/be/src/exec/hdfs

[Impala-CR](cdh5-trunk) IMPALA-2033: Netezza compatibility functions quote ident

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-2033: Netezza compatibility functions quote_ident .. Patch Set 4: Hi Shirish, have you had a chance to look at the comments? -- To view, visit http

[Impala-ASF-CR] IMPALA-1659: Netezza compatibility functions: metadata

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-1659: Netezza compatibility functions: metadata .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4063/1/be/src/exprs/utility-functions

Re: [DISCUSS] Branching for a release soon?

2016-08-19 Thread Tim Armstrong
hat is a bug fix. > > On Fri, Aug 19, 2016 at 9:01 AM, Tim Armstrong <tarmstr...@cloudera.com> > wrote: > > How do you plan to choose which commits to cherry-pick? Should we let you > > know if we think a patch should/shouldn't be part of the release? > > > &

Re: [DISCUSS] Branching for a release soon?

2016-08-19 Thread Tim Armstrong
How do you plan to choose which commits to cherry-pick? Should we let you know if we think a patch should/shouldn't be part of the release? On Fri, Aug 19, 2016 at 8:44 AM, Tom White wrote: > Thanks for volunteering to do the release Jim! The plan looks fine to me. > > Tom >

[Impala-ASF-CR] IMPALA-3943: Adhere to abort on error in ProcessFooter().

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3943: Adhere to abort_on_error in ProcessFooter(). .. Patch Set 7: (1 comment) Not this patch, but we should consider changing HdfsScanNode so

[Impala-ASF-CR] IMPALA-3342: Runtime profile TotalCpuTime should eliminate wait times

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3342: Runtime profile TotalCpuTime should eliminate wait times .. Patch Set 1: Unless I'm missing something, TotalCpuTime at the fragment level isn't

[Impala-ASF-CR] Add .clang-format for Impala's C++ style

2016-08-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Add .clang-format for Impala's C++ style .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/3886 To unsubscribe, visit http

[Impala-ASF-CR] Add functional and targeted perf tests for joins with empty builds

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Add functional and targeted perf tests for joins with empty builds .. Patch Set 3: Code-Review+2 carry +2 -- To view, visit http://gerrit.cloudera.org:8080

[Impala-ASF-CR] IMPALA-3090: always log memory limit errors

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3090: always log memory limit errors .. Patch Set 3: Code-Review+2 Rebase -- To view, visit http://gerrit.cloudera.org:8080/4049 To unsubscribe, visit

Re: [DISCUSS] Bylaw change to make branch creation or deletion lazy consensus

2016-08-18 Thread Tim Armstrong
Sounds reasonable to me. On Thu, Aug 18, 2016 at 4:04 PM, Jim Apple wrote: > http://gerrit.cloudera.org:8080/4053 > > I am proposing we change the bylaws so that a branch can be created or > deleted by lazy consensus of the PMC: "Lazy consensus requires no -1 > votes

[Impala-ASF-CR](asf-site) Update bylaws: Lazy Consensus for branch creation and deletion.

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Update bylaws: Lazy Consensus for branch creation and deletion. .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4053

[Impala-ASF-CR] Add functional and targeted perf tests for joins with empty builds

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Add functional and targeted perf tests for joins with empty builds .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4051/1/testdata/workloads

[Impala-ASF-CR] Add functional and targeted perf tests for joins with empty builds

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2). Change subject: Add functional and targeted perf tests for joins with empty builds .. Add functional and targeted perf tests for joins with empty builds I wrote these tests

[Impala-ASF-CR] Add functional and targeted perf tests for joins with empty builds

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4051 Change subject: Add functional and targeted perf tests for joins with empty builds .. Add functional and targeted perf tests for joins

[Impala-ASF-CR] IMPALA-(3895,3859): Don't log file data on parse errors

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-(3895,3859): Don't log file data on parse errors .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4020/2/testdata/workloads/functional

[Impala-ASF-CR] IMPALA-3090: always log memory limit errors

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3090: always log memory limit errors .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/4049/1/be/src/runtime/mem-tracker.h File be/src

[Impala-ASF-CR] IMPALA-3090: always log memory limit errors

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2). Change subject: IMPALA-3090: always log memory limit errors .. IMPALA-3090: always log memory limit errors Consistently log memory limit errors so that the error message contains

[Impala-ASF-CR] IMPALA-(3895,3859): Don't log file data on parse errors

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-(3895,3859): Don't log file data on parse errors .. Patch Set 2: (4 comments) It's kind of unfortunate to lose the diagnostic info if people are running

[Impala-ASF-CR] IMPALA-3090: always log memory limit errors

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new change for review. http://gerrit.cloudera.org:8080/4049 Change subject: IMPALA-3090: always log memory limit errors .. IMPALA-3090: always log memory limit errors Consistently log memory

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. Patch Set 7: I figured out what was going on with the profiles - the builder profile was being added

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#7). Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder The main outcome of this patch is to split out

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. Patch Set 5: (17 comments) Thanks for looking over the changes. I know it's a big patchset. Let me

[Impala-ASF-CR] IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#6). Change subject: IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder .. IMPALA-3567 Part 2, IMPALA-3899: factor out PHJ builder The main outcome of this patch is to split out

[Impala-ASF-CR] Add .clang-format for Impala's C++ style

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Add .clang-format for Impala's C++ style .. Patch Set 3: Shouldn't the ternary operator be affected by BreakBeforeTernaryOperators though? Or is that something

Re: Code formatting with clang-format

2016-08-18 Thread Tim Armstrong
but I also think we > > should be willing to update it sometimes. I think when we do update > > it, we don't need to do a bulk reformat. > > > > On Tue, Aug 16, 2016 at 9:06 AM, Tim Armstrong <tarmstr...@cloudera.com> > wrote: > >> +1 for automating thi

[Impala-ASF-CR] Add .clang-format for Impala's C++ style

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Add .clang-format for Impala's C++ style .. Patch Set 3: Code-Review+1 Sounds ok to me then -- To view, visit http://gerrit.cloudera.org:8080/3886

[Impala-ASF-CR] Add .clang-format for Impala's C++ style

2016-08-18 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. Change subject: Add .clang-format for Impala's C++ style .. Patch Set 2: I ran this on partitioned-aggregation-node.cc and mostly the changes were improvements (i.e

<    1   2   3   4   5   6   7   8   9   10   >