[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 06 Nov 2024 03:22:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. IMPALA-13502: Clean up around constructors Makes 'name' a required parameter around CreateBinaryPhiNode, CodegenNullPhiNode, and CodegenIsNullPhiNode to skip rare case where we generate a name. Uses raw pointers or const& rather than const shared_ptr& in some cases to simplify API. Removes unused TSaslServerTransport constructors. Reuses CharMemTrackerAllocator and OutboundRowBatch in TupleFileWriter. Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Reviewed-on: http://gerrit.cloudera.org:8080/22010 Reviewed-by: Riza Suminto Tested-by: Impala Public Jenkins --- M be/src/benchmarks/row-batch-serialize-benchmark.cc M be/src/codegen/codegen-anyval-read-write-info.h M be/src/codegen/codegen-anyval.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/hash-table.cc M be/src/exec/tuple-file-writer.cc M be/src/exec/tuple-file-writer.h M be/src/rpc/TAcceptQueueServer.cpp M be/src/rpc/TAcceptQueueServer.h M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/outbound-row-batch.h M be/src/runtime/row-batch-serialize-test.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/transport/TSaslServerTransport.cpp M be/src/transport/TSaslServerTransport.h 17 files changed, 71 insertions(+), 99 deletions(-) Approvals: Riza Suminto: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 05 Nov 2024 21:51:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11108/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 05 Nov 2024 21:53:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 8: > Patch Set 8: > > Tests encountered an instance of IMPALA-13500. Try rebasing. IMPALA-13507 might indirectly fix IMPALA-13500. -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 05 Nov 2024 18:23:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 8: Tests encountered an instance of IMPALA-13500. -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 05 Nov 2024 17:20:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Michael Smith has removed a vote on this change. Change subject: IMPALA-13502: Clean up around constructors .. Removed Verified-1 by Impala Public Jenkins -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 8: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11103/ -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 05 Nov 2024 04:26:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/17362/ : 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/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Nov 2024 23:25:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 7: -Verified Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/17360/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Nov 2024 23:14:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11102/ -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Nov 2024 22:58:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Hello Daniel Becker, Riza Suminto, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/22010 to look at the new patch set (#8). Change subject: IMPALA-13502: Clean up around constructors .. IMPALA-13502: Clean up around constructors Makes 'name' a required parameter around CreateBinaryPhiNode, CodegenNullPhiNode, and CodegenIsNullPhiNode to skip rare case where we generate a name. Uses raw pointers or const& rather than const shared_ptr& in some cases to simplify API. Removes unused TSaslServerTransport constructors. Reuses CharMemTrackerAllocator and OutboundRowBatch in TupleFileWriter. Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 --- M be/src/benchmarks/row-batch-serialize-benchmark.cc M be/src/codegen/codegen-anyval-read-write-info.h M be/src/codegen/codegen-anyval.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/hash-table.cc M be/src/exec/tuple-file-writer.cc M be/src/exec/tuple-file-writer.h M be/src/rpc/TAcceptQueueServer.cpp M be/src/rpc/TAcceptQueueServer.h M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/outbound-row-batch.h M be/src/runtime/row-batch-serialize-test.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/transport/TSaslServerTransport.cpp M be/src/transport/TSaslServerTransport.h 17 files changed, 71 insertions(+), 99 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/22010/8 -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11103/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Nov 2024 23:00:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11102/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Nov 2024 22:46:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Hello Daniel Becker, Riza Suminto, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/22010 to look at the new patch set (#7). Change subject: IMPALA-13502: Clean up around constructors .. IMPALA-13502: Clean up around constructors Makes 'name' a required parameter around CreateBinaryPhiNode, CodegenNullPhiNode, and CodegenIsNullPhiNode to skip rare case where we generate a name. Uses raw pointers or const& rather than const shared_ptr& in some cases to simplify API. Removes unused TSaslServerTransport constructors. Reuses CharMemTrackerAllocator and OutboundRowBatch in TupleFileWriter. Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 --- M be/src/benchmarks/row-batch-serialize-benchmark.cc M be/src/codegen/codegen-anyval-read-write-info.h M be/src/codegen/codegen-anyval.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/hash-table.cc M be/src/exec/tuple-file-writer.cc M be/src/exec/tuple-file-writer.h M be/src/rpc/TAcceptQueueServer.cpp M be/src/rpc/TAcceptQueueServer.h M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/outbound-row-batch.h M be/src/runtime/row-batch-serialize-test.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/transport/TSaslServerTransport.cpp M be/src/transport/TSaslServerTransport.h 17 files changed, 69 insertions(+), 98 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/22010/7 -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 7: (1 comment) gerrit-auto-critic failed. You can reproduce it locally using command: python2 bin/jenkins/critique-gerrit-review.py --dryrun To run it, you might need a virtual env with virtualenv installed. http://gerrit.cloudera.org:8080/#/c/22010/7/be/src/exec/tuple-file-writer.cc File be/src/exec/tuple-file-writer.cc: http://gerrit.cloudera.org:8080/#/c/22010/7/be/src/exec/tuple-file-writer.cc@89 PS7, Line 89: RETURN_IF_ERROR(row_batch->Serialize(out_batch_.get(), /* compression_scratch */ nullptr)); line too long (95 > 90) -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 04 Nov 2024 22:47:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 5: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11093/ -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Sat, 02 Nov 2024 04:34:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/17340/ : 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/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 22:58:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11093/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 22:48:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 5: Code-Review+2 LGTM, thanks! -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 22:42:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Hello Daniel Becker, Riza Suminto, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/22010 to look at the new patch set (#5). Change subject: IMPALA-13502: Clean up around constructors .. IMPALA-13502: Clean up around constructors Makes 'name' a required parameter around CreateBinaryPhiNode, CodegenNullPhiNode, and CodegenIsNullPhiNode to skip rare case where we generate a name. Uses raw pointers or const& rather than const shared_ptr& in some cases to simplify API. Removes unused TSaslServerTransport constructors. Reuses CharMemTrackerAllocator and OutboundRowBatch in TupleFileWriter. Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 --- M be/src/benchmarks/row-batch-serialize-benchmark.cc M be/src/codegen/codegen-anyval-read-write-info.h M be/src/codegen/codegen-anyval.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/hash-table.cc M be/src/exec/tuple-file-writer.cc M be/src/exec/tuple-file-writer.h M be/src/rpc/TAcceptQueueServer.cpp M be/src/rpc/TAcceptQueueServer.h M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/outbound-row-batch.h M be/src/runtime/row-batch-serialize-test.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/transport/TSaslServerTransport.cpp M be/src/transport/TSaslServerTransport.h 17 files changed, 66 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/22010/5 -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/22010/4/be/src/exec/tuple-file-writer.cc File be/src/exec/tuple-file-writer.cc: http://gerrit.cloudera.org:8080/#/c/22010/4/be/src/exec/tuple-file-writer.cc@83 PS4, Line 83: // serialize and wri > I think this OutboundRowBatch can also be reused? Done -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 22:31:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/22010/4/be/src/exec/tuple-file-writer.cc File be/src/exec/tuple-file-writer.cc: http://gerrit.cloudera.org:8080/#/c/22010/4/be/src/exec/tuple-file-writer.cc@83 PS4, Line 83: OutboundRowBatch out I think this OutboundRowBatch can also be reused? Note that RowBatch::Serialize calls output_batch->tuple_offsets_.clear() and output_batch->tuple_data_.resize(size). -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 22:22:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/17339/ : 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/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 22:16:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Hello Daniel Becker, Riza Suminto, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/22010 to look at the new patch set (#4). Change subject: IMPALA-13502: Clean up around constructors .. IMPALA-13502: Clean up around constructors Makes 'name' a required parameter around CreateBinaryPhiNode, CodegenNullPhiNode, and CodegenIsNullPhiNode to skip rare case where we generate a name. Uses raw pointers or const& rather than const shared_ptr& in some cases to simplify API. Removes unused TSaslServerTransport constructors. Reuses CharMemTrackerAllocator in TupleFileWriter. Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 --- M be/src/benchmarks/row-batch-serialize-benchmark.cc M be/src/codegen/codegen-anyval-read-write-info.h M be/src/codegen/codegen-anyval.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/hash-table.cc M be/src/exec/tuple-file-writer.cc M be/src/exec/tuple-file-writer.h M be/src/rpc/TAcceptQueueServer.cpp M be/src/rpc/TAcceptQueueServer.h M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/outbound-row-batch.h M be/src/runtime/row-batch-serialize-test.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/transport/TSaslServerTransport.cpp M be/src/transport/TSaslServerTransport.h 17 files changed, 57 insertions(+), 91 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/22010/4 -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/exec/tuple-file-writer.cc File be/src/exec/tuple-file-writer.cc: http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/exec/tuple-file-writer.cc@82 PS3, Line 82: // serialize and write row batch : OutboundRowBatch out(allocator_) > Yeah, looks like it. Done -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 21:49:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/exec/tuple-file-writer.cc File be/src/exec/tuple-file-writer.cc: http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/exec/tuple-file-writer.cc@82 PS3, Line 82: CharMemTrackerAllocator allocator(tracker_); : OutboundRowBatch out(allocator); > Can this reused between multiple Write() ? Yeah, looks like it. -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 21:44:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/codegen/codegen-anyval-read-write-info.h File be/src/codegen/codegen-anyval-read-write-info.h: http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/codegen/codegen-anyval-read-write-info.h@175 PS3, Line 175: llvm::PHINode* CodegenNullPhiNode( : llvm::Value* non_null_value, llvm::Value* null_value, const std::string& name); > Opening ./be/src/exec/CMakeFiles/Exec.dir/filter-context.cc.o, val_ptr_phi Ack http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/exec/tuple-file-writer.cc File be/src/exec/tuple-file-writer.cc: http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/exec/tuple-file-writer.cc@82 PS3, Line 82: CharMemTrackerAllocator allocator(tracker_); : OutboundRowBatch out(allocator); Can this reused between multiple Write() ? -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 21:41:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/codegen/codegen-anyval-read-write-info.h File be/src/codegen/codegen-anyval-read-write-info.h: http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/codegen/codegen-anyval-read-write-info.h@175 PS3, Line 175: llvm::PHINode* CodegenNullPhiNode( : llvm::Value* non_null_value, llvm::Value* null_value, const std::string& name); > They tend to be context-specific, I'm not sure it makes sense to couple the Opening ./be/src/exec/CMakeFiles/Exec.dir/filter-context.cc.o, val_ptr_phi appears exactly once. -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 20:40:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/codegen/codegen-anyval-read-write-info.h File be/src/codegen/codegen-anyval-read-write-info.h: http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/codegen/codegen-anyval-read-write-info.h@175 PS3, Line 175: llvm::PHINode* CodegenNullPhiNode( : llvm::Value* non_null_value, llvm::Value* null_value, const std::string& name); > nit: They tend to be context-specific, I'm not sure it makes sense to couple them. And they share the same entry in .data; I'll double check that actually happens. -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 18:14:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 3: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/codegen/codegen-anyval-read-write-info.h File be/src/codegen/codegen-anyval-read-write-info.h: http://gerrit.cloudera.org:8080/#/c/22010/3/be/src/codegen/codegen-anyval-read-write-info.h@175 PS3, Line 175: llvm::PHINode* CodegenNullPhiNode( : llvm::Value* non_null_value, llvm::Value* null_value, const std::string& name); nit: FilterContext::CodegenEval, FilterContext::CodegenInsert, and KrpcDataStreamSenderConfig::CodegenHashRow all call this with same "val_ptr_phi" name. CodegenAnyVal::CreateFromReadWriteInfo and HashTableCtx::CodegenEvalRow has same "is_null_phi" name. Can that names be static constexpr? They seem significant. -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 01 Nov 2024 16:01:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/17336/ : 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/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 01 Nov 2024 00:15:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22010 ) Change subject: IMPALA-13502: Clean up around constructors .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/17331/ : 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/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 31 Oct 2024 19:16:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13502: Clean up around constructors
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22010 Change subject: IMPALA-13502: Clean up around constructors .. IMPALA-13502: Clean up around constructors Makes 'name' a required parameter around CreateBinaryPhiNode, CodegenNullPhiNode, and CodegenIsNullPhiNode to skip rare case where we generate a name. Uses raw pointers or const& rather than const shared_ptr& in some cases to simplify API. Removes unused TSaslServerTransport constructors. Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 --- M be/src/benchmarks/row-batch-serialize-benchmark.cc M be/src/codegen/codegen-anyval-read-write-info.h M be/src/codegen/codegen-anyval.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/hash-table.cc M be/src/exec/tuple-file-writer.cc M be/src/rpc/TAcceptQueueServer.cpp M be/src/rpc/TAcceptQueueServer.h M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/outbound-row-batch.h M be/src/runtime/row-batch-serialize-test.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/transport/TSaslServerTransport.cpp M be/src/transport/TSaslServerTransport.h 16 files changed, 54 insertions(+), 90 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/22010/1 -- To view, visit http://gerrit.cloudera.org:8080/22010 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I432108a6b26f42664163e1d1fa137dbc7d14 Gerrit-Change-Number: 22010 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith
