[kudu-CR] [client] updated multi-row transaction API
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. [client] updated multi-row transaction API This patch removes KuduTransactionSerializer and switches to using KuduTransaction::SerializationOptions instead. Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Reviewed-on: http://gerrit.cloudera.org:8080/16948 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins Reviewed-by: Hao Hao Reviewed-by: Grant Henke --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/transaction-internal.cc M src/kudu/client/transaction-internal.h 5 files changed, 170 insertions(+), 163 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified Hao Hao: Looks good to me, approved Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong
[kudu-CR] [client] updated multi-row transaction API
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. Patch Set 3: Tagging Tim for context. -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 14 Jan 2021 21:06:24 + Gerrit-HasComments: No
[kudu-CR] [client] updated multi-row transaction API
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Jan 2021 21:03:50 + Gerrit-HasComments: No
[kudu-CR] [client] updated multi-row transaction API
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Jan 2021 21:01:43 + Gerrit-HasComments: No
[kudu-CR] [client] updated multi-row transaction API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16948/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/16948/1/src/kudu/client/client.h@557 PS1, Line 557: Serialize(s > I'm fine with either approach. FWIW we make that same exception for ListTab Ah, indeed: ListTables() is another exception. Thanks for the pointer. -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Jan 2021 20:51:35 + Gerrit-HasComments: Yes
[kudu-CR] [client] updated multi-row transaction API
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. Patch Set 3: Code-Review+2 LGTM, though would be good to get another look from Grant. -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Jan 2021 20:41:19 + Gerrit-HasComments: No
[kudu-CR] [client] updated multi-row transaction API
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16948 to look at the new patch set (#3). Change subject: [client] updated multi-row transaction API .. [client] updated multi-row transaction API This patch removes KuduTransactionSerializer and switches to using KuduTransaction::SerializationOptions instead. Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/transaction-internal.cc M src/kudu/client/transaction-internal.h 5 files changed, 170 insertions(+), 163 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/16948/3 -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] updated multi-row transaction API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16948/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/16948/1/src/kudu/client/client.h@557 PS1, Line 557: Serialize(s > We could have two methods like that, but I think it looks ugly and even con I guess we can make an exception w.r.t. code style conventions because the code style convention here contradicts the simple usability criteria. OK, I'll post new PS for this patch. -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Jan 2021 19:04:50 + Gerrit-HasComments: Yes
[kudu-CR] [client] updated multi-row transaction API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16948/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/16948/1/src/kudu/client/client.h@557 PS1, Line 557: Serialize(s > Can we have Serialize(options, _token) and Serialize(_token)? I pre We could have two methods like that, but I think it looks ugly and even confusing. I'd rather keep it as is, or, if really necessary, ignore the style convention and introduce the options parameter to be by default. -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Jan 2021 18:53:50 + Gerrit-HasComments: Yes
[kudu-CR] [client] updated multi-row transaction API
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16948 ) Change subject: [client] updated multi-row transaction API .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16948/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/16948/1/src/kudu/client/client.h@557 PS1, Line 557: Serialize(s > I wanted to be able to call this method as just Serialize(_token), so t Can we have Serialize(options, _token) and Serialize(_token)? I prefer the options as an argument as well. -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 14 Jan 2021 14:39:43 + Gerrit-HasComments: Yes
[kudu-CR] [client] updated multi-row transaction API
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16948 to look at the new patch set (#2). Change subject: [client] updated multi-row transaction API .. [client] updated multi-row transaction API This patch removes KuduTransactionSerializer and switches to using KuduTransaction::SerializationOptions instead. Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/transaction-internal.cc M src/kudu/client/transaction-internal.h 5 files changed, 170 insertions(+), 163 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/16948/2 -- To view, visit http://gerrit.cloudera.org:8080/16948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id646c41d60f385f079ebfaec2daf7c2a108306b2 Gerrit-Change-Number: 16948 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
