[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 10 Aug 2018 09:22:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Reviewed-on: http://gerrit.cloudera.org:8080/11156 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h M be/src/runtime/tuple.h 6 files changed, 408 insertions(+), 137 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 10 Aug 2018 06:07:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 3: Hit IMPALA-7415 -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 10 Aug 2018 06:07:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2973/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 10 Aug 2018 06:07:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2971/ -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 10 Aug 2018 03:11:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2971/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 23:46:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 23:46:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 09 Aug 2018 23:45:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11156 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 2: This is a clean pick of https://gerrit.cloudera.org/#/c/11007/ -- To view, visit http://gerrit.cloudera.org:8080/11156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f5b8a418d4816f603a64da6287ec392dbd4603f Gerrit-Change-Number: 11156 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 08 Aug 2018 21:08:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 14: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 31 Jul 2018 02:25:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Reviewed-on: http://gerrit.cloudera.org:8080/11007 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h M be/src/runtime/tuple.h 6 files changed, 408 insertions(+), 137 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 15 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2881/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 22:51:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 12: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/101/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 22:37:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 14: No Builds Executed -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 22:34:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 14: Code-Review+2 Fix tests, will wait for jenkins restart to merge -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 19:00:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#14). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h M be/src/runtime/tuple.h 6 files changed, 408 insertions(+), 137 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/14 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 17:28:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 13: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2877/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 17:28:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 12: Code-Review+2 Carry -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 17:28:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 12: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/101/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 17:28:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/11007/11/be/src/runtime/buffered-tuple-stream-test.cc File be/src/runtime/buffered-tuple-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/11007/11/be/src/runtime/buffered-tuple-stream-test.cc@961 PS11, Line 961: EXPECT_EQ(*static_cast(in_row->GetTuple(0)->GetSlot(slot_offset)), > optional: Maybe a GetIntSlot() function could be added to Tuple to avoid ca Done -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 17:28:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#12). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h M be/src/runtime/tuple.h 6 files changed, 409 insertions(+), 137 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/12 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 11: fyi, the builds probably failed because of https://issues.apache.org/jira/browse/IMPALA-7328 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 15:37:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 11: Code-Review+2 (1 comment) Thanks for the refactors! I have rerun the failed jenkins job as I hope that it failed due to unrelated flakiness. http://gerrit.cloudera.org:8080/#/c/11007/11/be/src/runtime/buffered-tuple-stream-test.cc File be/src/runtime/buffered-tuple-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/11007/11/be/src/runtime/buffered-tuple-stream-test.cc@961 PS11, Line 961: EXPECT_EQ(*static_cast(in_row->GetTuple(0)->GetSlot(slot_offset)), optional: Maybe a GetIntSlot() function could be added to Tuple to avoid casting here. Similar functions already exist for some types like string and bigint. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 15:27:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 11: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2876/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 15:20:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 11: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2875/ -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 10:02:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/97/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 07:14:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 11: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2875/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 06:40:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 11: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/97/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 30 Jul 2018 06:40:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#11). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h 5 files changed, 401 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/11 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/96/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 28 Jul 2018 03:56:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11007 )
Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
..
Patch Set 9:
(5 comments)
Lol, hopefully with some refactoring it's a little more readable - I did just
put it together quickly
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@863
PS9, Line 863: void SimpleTupleStreamTest::TestFlushResourcesReadWrite(bool
pin_stream, bool attach_on_read) {
> nit: long line
Done
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@913
PS9, Line 913: for (int j = 0; j < out_batch->num_rows(); ++j) {
> style: Level 5 loop nesting seems a bit too deep to me - can you move some
Done
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@916
PS9, Line 916:
*static_cast(out_batch->GetRow(j)->GetTuple(0)->GetSlot(slot_offset)));
> nit: long line
Done
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.h@395
PS9, Line 395: AttachBuffer
> style: The name suggests to me that we are attaching a buffer TO the page h
I used the first one.
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.cc
File be/src/runtime/buffered-tuple-stream.cc:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.cc@784
PS9, Line 784: batch->MarkFlushResources(); // TODO: remove
> This seems to be called at the same conditions as line 805. If this is inte
Oops, removed now.
--
To view, visit http://gerrit.cloudera.org:8080/11007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5
Gerrit-Change-Number: 11007
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Sat, 28 Jul 2018 03:14:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 10: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/96/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 28 Jul 2018 03:14:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#10). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h 5 files changed, 392 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/10 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11007 )
Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
..
Patch Set 9:
(5 comments)
The new test seems mind-bending this late at night. I will give it another run
on Monday. I have some nits and style comments till then.
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@863
PS9, Line 863: void SimpleTupleStreamTest::TestFlushResourcesReadWrite(bool
pin_stream, bool attach_on_read) {
nit: long line
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@913
PS9, Line 913: for (int j = 0; j < out_batch->num_rows(); ++j) {
style: Level 5 loop nesting seems a bit too deep to me - can you move some of
the inner loops into separate functions? For example the verifying logic from
line 908 to 918 seems a good candidate to me. This is definitely subjective, so
feel free to keep it as it is.
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@916
PS9, Line 916:
*static_cast(out_batch->GetRow(j)->GetTuple(0)->GetSlot(slot_offset)));
nit: long line
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.h@395
PS9, Line 395: AttachBuffer
style: The name suggests to me that we are attaching a buffer TO the page here.
Some names that came to my mind are "AttachBufferToBatch", "ExtractBuffer" or
"TakeBuffer".
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.cc
File be/src/runtime/buffered-tuple-stream.cc:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.cc@784
PS9, Line 784: batch->MarkFlushResources(); // TODO: remove
This seems to be called at the same conditions as line 805. If this is
intentional, can you explain why?
--
To view, visit http://gerrit.cloudera.org:8080/11007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5
Gerrit-Change-Number: 11007
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 27 Jul 2018 23:03:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/90/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 21:30:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 9: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/90/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 20:56:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc File be/src/runtime/buffered-tuple-stream.cc: http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc@763 PS8, Line 763: has_read_write_page > I was able to hit a DCHECK in NextReadPage() with a new test, will work on I added a regression test that can trigger the two bugs you identified and fixed them. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 20:56:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#9). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h 5 files changed, 383 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/9 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc File be/src/runtime/buffered-tuple-stream.cc: http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc@763 PS8, Line 763: has_read_write_page > I think I see your point - in that case on the next GetNext() call we go do I was able to hit a DCHECK in NextReadPage() with a new test, will work on a fix. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 01:08:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11007 )
Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
..
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc
File be/src/runtime/buffered-tuple-stream.cc:
http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc@763
PS8, Line 763: has_read_write_page
> Is this always safe? If next row will not fit into the page then the write
I think I see your point - in that case on the next GetNext() call we go down
the branch below, right?
if (UNLIKELY(read_page_ == pages_.end()
|| read_page_rows_returned_ == read_page_->num_rows)) {
--
To view, visit http://gerrit.cloudera.org:8080/11007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5
Gerrit-Change-Number: 11007
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 26 Jul 2018 23:33:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc File be/src/runtime/buffered-tuple-stream.cc: http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc@763 PS8, Line 763: has_read_write_page Is this always safe? If next row will not fit into the page then the write page will be advanced on write. So calling AddRow() and GetNext() will decrease the pin count of the current page to 0. ReadWrite streams are only used by AnalyticEvalNode() - I looked at it a bit but I am not sure yet whether this can cause a problem or not. My idea for a safe solution is that if attach_on_read_ is false, then MarkFlushResources() has to be called for read_write pages, while if attach_on_read_ is true, then the page has to be attached to the RowBatch in the next GetNext() call before calling NextReadPage(). -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 26 Jul 2018 13:52:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/56/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 25 Jul 2018 18:54:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/55/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 25 Jul 2018 18:47:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 25 Jul 2018 18:32:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 7: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/56/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 25 Jul 2018 18:14:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 8: Code-Review+1 carry Csaba +1 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 25 Jul 2018 18:14:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added a unit test to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h 5 files changed, 233 insertions(+), 122 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/7 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 8: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/55/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 25 Jul 2018 18:12:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#8). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added a unit test to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h 5 files changed, 241 insertions(+), 124 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/8 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/42/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 24 Jul 2018 23:29:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/11007/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11007/4//COMMIT_MSG@7 PS4, Line 7: TS > Does this patch address joins? Yeah I guess that's a bit confusing, the BufferedTupleStream is used by HashJoin. http://gerrit.cloudera.org:8080/#/c/11007/4/be/src/runtime/buffered-tuple-stream-test.cc File be/src/runtime/buffered-tuple-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/11007/4/be/src/runtime/buffered-tuple-stream-test.cc@678 PS4, Line 678: // on previous GetNext() calls. : ASSERT_EQ(0, stream.BytesPinned(false)); : > Is this comment still accurate? It is 0 now Done -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 24 Jul 2018 23:04:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 5: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/42/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 24 Jul 2018 23:04:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#5). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added a unit test to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h 5 files changed, 235 insertions(+), 122 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/5 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
