[kudu-CR] [tserver] Move cell and probe key size checking to prepare phase
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13470 ) Change subject: [tserver] Move cell and probe key size checking to prepare phase .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c Gerrit-Change-Number: 13470 Gerrit-PatchSet: 8 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Tue, 30 Jul 2019 04:22:17 + Gerrit-HasComments: No
[kudu-CR] [tserver] Move cell and probe key size checking to prepare phase
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13470 to look at the new patch set (#8). Change subject: [tserver] Move cell and probe key size checking to prepare phase .. [tserver] Move cell and probe key size checking to prepare phase It's inefficient to check cell and probe key size in apply phase, because we have to lock row key and construct new Slices for every binary type of columns. This patch move this lightweight checking to prepare phase when decoding values from protobuf and before locking row keys. Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c --- M src/kudu/clock/clock.h M src/kudu/common/row_operations-test.cc M src/kudu/common/row_operations.cc M src/kudu/common/row_operations.h M src/kudu/tablet/row_op.cc M src/kudu/tablet/row_op.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_bootstrap.cc 9 files changed, 240 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/13470/8 -- To view, visit http://gerrit.cloudera.org:8080/13470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c Gerrit-Change-Number: 13470 Gerrit-PatchSet: 8 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13833 ) Change subject: KUDU-2881 Support create/drop range partition by command line .. KUDU-2881 Support create/drop range partition by command line Sometimes we need to drop the range partition and then recreate it in order to rewrite in case that the partition was written wrongly. This patch supports to add/drop range partition by command line. The command can be used as: 1. kudu table add_range_partition [-lower_bound_type] [-upper_bound_type] 2. kudu table drop_range_partition [-lower_bound_type] [-upper_bound_type] The parameters of lower_bound and upper_bound can be empty, which means we will use the default value. If both these two parameters are empty, it means the range partition is unbounded. The parameters of lower_bound_type and upper_bound_type is optional. If these two parameters are not specified, the default value are INCLUSIVE_BOUND and EXCLUSIVE_BOUND respectively. These two parameters are both case-insensitive. Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Reviewed-on: http://gerrit.cloudera.org:8080/13833 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 752 insertions(+), 0 deletions(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 20 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin
[kudu-CR] Add the missing right parenthesis
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13953 ) Change subject: Add the missing right parenthesis .. Add the missing right parenthesis Change-Id: I953f29afa5d8060b84748abc38a1ec56c963cb6e Reviewed-on: http://gerrit.cloudera.org:8080/13953 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13953 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I953f29afa5d8060b84748abc38a1ec56c963cb6e Gerrit-Change-Number: 13953 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Add the missing right parenthesis
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13953 ) Change subject: Add the missing right parenthesis .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13953 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I953f29afa5d8060b84748abc38a1ec56c963cb6e Gerrit-Change-Number: 13953 Gerrit-PatchSet: 1 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 30 Jul 2019 04:05:06 + Gerrit-HasComments: No
[kudu-CR] [tserver] Move cell and probe key size checking to prepare phase
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13470 ) Change subject: [tserver] Move cell and probe key size checking to prepare phase .. Patch Set 7: Code-Review+1 (1 comment) Todd, do you want to take a look too? http://gerrit.cloudera.org:8080/#/c/13470/7/src/kudu/common/row_operations.h File src/kudu/common/row_operations.h: http://gerrit.cloudera.org:8080/#/c/13470/7/src/kudu/common/row_operations.h@111 PS7, Line 111: returm return -- To view, visit http://gerrit.cloudera.org:8080/13470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c Gerrit-Change-Number: 13470 Gerrit-PatchSet: 7 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Tue, 30 Jul 2019 04:04:45 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] Move cell and probe key size checking to prepare phase
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13470 ) Change subject: [tserver] Move cell and probe key size checking to prepare phase .. Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.h File src/kudu/common/row_operations.h: http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.h@107 PS6, Line 107: // Read one row's column data from 'src_', read result is stored in 'slice'. : // Return bad Status if data is corrupt. : // NOTE: If 'row_status' is not nullptr, column data validate will be performed, : // and if column data validate error (i.e. column size ex > Should also add how setting row_status to nullptr or not will affect column Done http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.cc File src/kudu/common/row_operations.cc: http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.cc@421 PS6, Line 421: if (PREDICT_FALSE(!row_status.ok())) op->SetFailureStatusOnce(row_status); > Could PREDICT_FALSE here. Done http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.cc@518 PS6, Line 518: if (PREDICT_FALSE(!row_status.ok())) op->SetFailureStatusOnce(row_status); > Could PREDICT_FALSE here. Done -- To view, visit http://gerrit.cloudera.org:8080/13470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c Gerrit-Change-Number: 13470 Gerrit-PatchSet: 7 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Tue, 30 Jul 2019 03:42:04 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] Move cell and probe key size checking to prepare phase
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13470 to look at the new patch set (#7). Change subject: [tserver] Move cell and probe key size checking to prepare phase .. [tserver] Move cell and probe key size checking to prepare phase It's inefficient to check cell and probe key size in apply phase, because we have to lock row key and construct new Slices for every binary type of columns. This patch move this lightweight checking to prepare phase when decoding values from protobuf and before locking row keys. Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c --- M src/kudu/clock/clock.h M src/kudu/common/row_operations-test.cc M src/kudu/common/row_operations.cc M src/kudu/common/row_operations.h M src/kudu/tablet/row_op.cc M src/kudu/tablet/row_op.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_bootstrap.cc 9 files changed, 240 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/13470/7 -- To view, visit http://gerrit.cloudera.org:8080/13470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c Gerrit-Change-Number: 13470 Gerrit-PatchSet: 7 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13833 ) Change subject: KUDU-2881 Support create/drop range partition by command line .. Patch Set 19: Code-Review+2 Thanks for the contribution! :) -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 19 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin Gerrit-Comment-Date: Tue, 30 Jul 2019 03:16:02 + Gerrit-HasComments: No
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
honeyhexin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13833 ) Change subject: KUDU-2881 Support create/drop range partition by command line .. Patch Set 19: (2 comments) http://gerrit.cloudera.org:8080/#/c/13833/18/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/13833/18/src/kudu/tools/kudu-admin-test.cc@2374 PS18, Line 2374:RETURN_NOT_OK(result); : } > We should probably RETURN_NOT_OK(result) here, since all we know at this po Done http://gerrit.cloudera.org:8080/#/c/13833/18/src/kudu/tools/kudu-admin-test.cc@2573 PS18, Line 2573: > nit: I don't think you need to even define a variable for this. You could j Done -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 19 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin Gerrit-Comment-Date: Tue, 30 Jul 2019 03:07:26 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Yao Xu, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13833 to look at the new patch set (#19). Change subject: KUDU-2881 Support create/drop range partition by command line .. KUDU-2881 Support create/drop range partition by command line Sometimes we need to drop the range partition and then recreate it in order to rewrite in case that the partition was written wrongly. This patch supports to add/drop range partition by command line. The command can be used as: 1. kudu table add_range_partition [-lower_bound_type] [-upper_bound_type] 2. kudu table drop_range_partition [-lower_bound_type] [-upper_bound_type] The parameters of lower_bound and upper_bound can be empty, which means we will use the default value. If both these two parameters are empty, it means the range partition is unbounded. The parameters of lower_bound_type and upper_bound_type is optional. If these two parameters are not specified, the default value are INCLUSIVE_BOUND and EXCLUSIVE_BOUND respectively. These two parameters are both case-insensitive. Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 752 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13833/19 -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 19 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin
[kudu-CR] Add the missing right parenthesis
ZhangYao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13953 Change subject: Add the missing right parenthesis .. Add the missing right parenthesis Change-Id: I953f29afa5d8060b84748abc38a1ec56c963cb6e --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/13953/1 -- To view, visit http://gerrit.cloudera.org:8080/13953 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I953f29afa5d8060b84748abc38a1ec56c963cb6e Gerrit-Change-Number: 13953 Gerrit-PatchSet: 1 Gerrit-Owner: ZhangYao
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13833 )
Change subject: KUDU-2881 Support create/drop range partition by command line
..
Patch Set 18: Code-Review+1
(2 comments)
A couple minor nits, otherwise, looks good to me!
http://gerrit.cloudera.org:8080/#/c/13833/18/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/13833/18/src/kudu/tools/kudu-admin-test.cc@2374
PS18, Line 2374: }
: RETUR
We should probably RETURN_NOT_OK(result) here, since all we know at this point
is that it's not an IOError.
http://gerrit.cloudera.org:8080/#/c/13833/18/src/kudu/tools/kudu-admin-test.cc@2573
PS18, Line 2573: out_of_range_rows_to_insert
nit: I don't think you need to even define a variable for this. You could just
do:
check_bounds("[100]", "[200]", "INCLUSIVE_BOUND", "EXCLUSIVE_BOUND", 100, 100,
{ 99, 200 });
Same elsewhere.
--
To view, visit http://gerrit.cloudera.org:8080/13833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c
Gerrit-Change-Number: 13833
Gerrit-PatchSet: 18
Gerrit-Owner: honeyhexin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yao Xu
Gerrit-Reviewer: honeyhexin
Gerrit-Comment-Date: Tue, 30 Jul 2019 02:14:57 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
honeyhexin has posted comments on this change. (
http://gerrit.cloudera.org:8080/13833 )
Change subject: KUDU-2881 Support create/drop range partition by command line
..
Patch Set 18:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13833/17/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/13833/17/src/kudu/tools/kudu-admin-test.cc@2372
PS17, Line 2372: return Status::NotFound(Substitute("No range partition
covers the insert value $0", i));
> Also don't need to explicitly close the session.
Done
--
To view, visit http://gerrit.cloudera.org:8080/13833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c
Gerrit-Change-Number: 13833
Gerrit-PatchSet: 18
Gerrit-Owner: honeyhexin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yao Xu
Gerrit-Reviewer: honeyhexin
Gerrit-Comment-Date: Mon, 29 Jul 2019 21:38:50 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Yao Xu, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13833 to look at the new patch set (#18). Change subject: KUDU-2881 Support create/drop range partition by command line .. KUDU-2881 Support create/drop range partition by command line Sometimes we need to drop the range partition and then recreate it in order to rewrite in case that the partition was written wrongly. This patch supports to add/drop range partition by command line. The command can be used as: 1. kudu table add_range_partition [-lower_bound_type] [-upper_bound_type] 2. kudu table drop_range_partition [-lower_bound_type] [-upper_bound_type] The parameters of lower_bound and upper_bound can be empty, which means we will use the default value. If both these two parameters are empty, it means the range partition is unbounded. The parameters of lower_bound_type and upper_bound_type is optional. If these two parameters are not specified, the default value are INCLUSIVE_BOUND and EXCLUSIVE_BOUND respectively. These two parameters are both case-insensitive. Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 771 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13833/18 -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 18 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin
[kudu-CR] [tserver] Move cell and probe key size checking to prepare phase
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13470 ) Change subject: [tserver] Move cell and probe key size checking to prepare phase .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.h File src/kudu/common/row_operations.h: http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.h@107 PS6, Line 107: // Read one row's column data from 'src_', read result is stored in 'slice'. : // Return bad Status if data is corrupt. : // NOTE: If column data validate error (i.e. column size exceed the limit), only : // set bad Status to 'row_status', and returm Status::OK. Should also add how setting row_status to nullptr or not will affect column size checking. http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc File src/kudu/common/row_operations.cc: http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc@605 PS5, Line 605: RETURN_NOT_OK(op->split_row->Set(static_cast(tablet_col_idx), data)); > Not check, because it's a split key, which will not stored in database. Right, and a split key will get validated when, after it's been serialized into a PartitionPB and stored in a SysTabletsEntryPB, the master tablet writes the SysTabletsEntryPB. At that point, the entire SysTabletsEntryPB is written out as a STRING and will get checked via the standard INSERT or UPDATE code paths. http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.cc File src/kudu/common/row_operations.cc: http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.cc@421 PS6, Line 421: if (!row_status.ok()) op->SetFailureStatusOnce(row_status); Could PREDICT_FALSE here. http://gerrit.cloudera.org:8080/#/c/13470/6/src/kudu/common/row_operations.cc@518 PS6, Line 518: if (!row_status.ok()) op->SetFailureStatusOnce(row_status); Could PREDICT_FALSE here. -- To view, visit http://gerrit.cloudera.org:8080/13470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c Gerrit-Change-Number: 13470 Gerrit-PatchSet: 6 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 29 Jul 2019 18:37:05 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13833 ) Change subject: KUDU-2881 Support create/drop range partition by command line .. Patch Set 17: Code-Review+1 (2 comments) Andrew this looks good to me; would you like to take another look? http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/kudu-admin-test.cc@2426 PS15, Line 2426: > As we have discussed offline, keep the logic here. To add context, honeyhexin pointed out that other tests in kudu-admin-test.cc use this style (ASSERT_TRUE on s.ok()), and he'd prefer to be consistent with the rest of the file. http://gerrit.cloudera.org:8080/#/c/13833/17/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/13833/17/src/kudu/tools/kudu-admin-test.cc@2372 PS17, Line 2372: RETURN_NOT_OK(session->Close()); Also don't need to explicitly close the session. -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 17 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin Gerrit-Comment-Date: Mon, 29 Jul 2019 18:23:02 + Gerrit-HasComments: Yes
[kudu-CR] [client] Deprecated TabletLocationsPB.ReplicaPB message in client
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13908 ) Change subject: [client] Deprecated TabletLocationsPB.ReplicaPB message in client .. [client] Deprecated TabletLocationsPB.ReplicaPB message in client In this patch, I removed the dependency on it from the client. It's only used for backward compatibility with RPC. In the future we will completely deprecated TabletLocationsPB.ReplicaPB message. Change-Id: I18272274f07d5ae8e9f6b9572f9900aa8df27bef Reviewed-on: http://gerrit.cloudera.org:8080/13908 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ProtobufUtils.java M src/kudu/client/client.cc M src/kudu/client/meta_cache.cc M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/delete_table-itest.cc M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/master_sentry-itest.cc M src/kudu/integration-tests/raft_config_change-itest.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/integration-tests/table_locations-itest.cc M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/integration-tests/tombstoned_voting-itest.cc M src/kudu/integration-tests/ts_itest-base.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/rebalancer_tool-test.cc 27 files changed, 315 insertions(+), 236 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I18272274f07d5ae8e9f6b9572f9900aa8df27bef Gerrit-Change-Number: 13908 Gerrit-PatchSet: 10 Gerrit-Owner: Yao Xu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yao Xu
[kudu-CR] [client] Deprecated TabletLocationsPB.ReplicaPB message in client
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13908 ) Change subject: [client] Deprecated TabletLocationsPB.ReplicaPB message in client .. Patch Set 9: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/13908/8/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: http://gerrit.cloudera.org:8080/#/c/13908/8/src/kudu/client/meta_cache.cc@198 PS8, Line 198: replicas.push_back(replica); > > Hmm, guess I was wrong about std::move here and below. Yeah that's right. I thought there was an std::string in there (UUID) but I was wrong. -- To view, visit http://gerrit.cloudera.org:8080/13908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18272274f07d5ae8e9f6b9572f9900aa8df27bef Gerrit-Change-Number: 13908 Gerrit-PatchSet: 9 Gerrit-Owner: Yao Xu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yao Xu Gerrit-Comment-Date: Mon, 29 Jul 2019 18:21:00 + Gerrit-HasComments: Yes
[kudu-CR] [common] add FindColumn for Schema
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13936 ) Change subject: [common] add FindColumn for Schema .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iede12b95b774754f914295cecd2f6797008fed46 Gerrit-Change-Number: 13936 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Mon, 29 Jul 2019 18:17:56 + Gerrit-HasComments: No
[kudu-CR] KUDU-1938 Add non-copy setters to partial row pt 3
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/13928 ) Change subject: KUDU-1938 Add non-copy setters to partial row pt 3 .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/13928/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13928/3//COMMIT_MSG@14 PS3, Line 14: to already be truncated (which it is in Impala's case) and only check > More context on bytes vs. characters (from part 1 of this series): https:// We do treat it as UTF8 characters, I didn't know Impala treated them differently. For some additional context, I did some testing, Oracle treats lengths based as either bytes or characters based on the NLS_LENGTH_SEMANTICS parameter, but it can be explicitly set in table creation time (e.g. CHAR(10 CHAR) or CHAR(10 BYTE)). MySQL is supposed to treat them as characters in recent versions, but in my version at least I couldn't make it work. Postgres treats them as characters. Anyway, if Impala treats them as bytes, it won't fail here as the length will be always <= than the allowed max length. http://gerrit.cloudera.org:8080/#/c/13928/3//COMMIT_MSG@17 PS3, Line 17: to avoid having to count each character manually. > is the unicode character counting not already fast-pathed for the ASCII sub We could do this, but & 0x8080808080808080 would only tell us whether it has UTF8 characters and it would still need to check the whole string. In this case I decided to sacrifice absolute correctness for performance, let me know if I should still check properly. The & 0x8080808080808080 is a good idea to optimize where I actually count the characters and thinking about it gave me an idea to optimize it a bit better even, I'll do some perf tests and submit another patch with the optimizations later if it makes sense. -- To view, visit http://gerrit.cloudera.org:8080/13928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f2aba098d649eb94e0314f6606cc33600e8d766 Gerrit-Change-Number: 13928 Gerrit-PatchSet: 3 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 29 Jul 2019 16:02:14 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Yao Xu, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13833 to look at the new patch set (#17). Change subject: KUDU-2881 Support create/drop range partition by command line .. KUDU-2881 Support create/drop range partition by command line Sometimes we need to drop the range partition and then recreate it in order to rewrite in case that the partition was written wrongly. This patch supports to add/drop range partition by command line. The command can be used as: 1. kudu table add_range_partition [-lower_bound_type] [-upper_bound_type] 2. kudu table drop_range_partition [-lower_bound_type] [-upper_bound_type] The parameters of lower_bound and upper_bound can be empty, which means we will use the default value. If both these two parameters are empty, it means the range partition is unbounded. The parameters of lower_bound_type and upper_bound_type is optional. If these two parameters are not specified, the default value are INCLUSIVE_BOUND and EXCLUSIVE_BOUND respectively. These two parameters are both case-insensitive. Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 772 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13833/17 -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 17 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
honeyhexin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13833 ) Change subject: KUDU-2881 Support create/drop range partition by command line .. Patch Set 11: (6 comments) http://gerrit.cloudera.org:8080/#/c/13833/15//COMMIT_MSG Commit Message: PS15: > Could you restore the commit message indentation as it was before? git will Done http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/kudu-admin-test.cc@2350 PS15, Line 2350: NO_FATALS(BuildAndStart()); > This function should be declared in an anonymous namespace, or declared sta Done http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/kudu-admin-test.cc@2372 PS15, Line 2372: : // At first, the range partition is un > This isn't necessary; what is being flushed here? Done http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/kudu-admin-test.cc@2426 PS15, Line 2426: // add or drop range partition. > ASSERT_OK(s) << ... would be more idiomatic. As we have discussed offline, keep the logic here. http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/kudu-admin-test.cc@2529 PS15, Line 2529: : // Drop the range partition ad > You can combine this into one line. Done http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/13833/15/src/kudu/tools/tool_action_table.cc@32 PS15, Line 32: #include > warning: #includes are not sorted properly [llvm-include-order] Done -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 11 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin Gerrit-Comment-Date: Mon, 29 Jul 2019 08:30:30 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2881 Support create/drop range partition by command line
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Yao Xu, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13833 to look at the new patch set (#16). Change subject: KUDU-2881 Support create/drop range partition by command line .. KUDU-2881 Support create/drop range partition by command line Sometimes we need to drop the range partition and then recreate it in order to rewrite in case that the partition was written wrongly. This patch supports to add/drop range partition by command line. The command can be used as: 1. kudu table add_range_partition [-lower_bound_type] [-upper_bound_type] 2. kudu table drop_range_partition [-lower_bound_type] [-upper_bound_type] The parameters of lower_bound and upper_bound can be empty, which means we will use the default value. If both these two parameters are empty, it means the range partition is unbounded. The parameters of lower_bound_type and upper_bound_type is optional. If these two parameters are not specified, the default value are INCLUSIVE_BOUND and EXCLUSIVE_BOUND respectively. These two parameters are both case-insensitive. Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_table.cc 4 files changed, 773 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/13833/16 -- To view, visit http://gerrit.cloudera.org:8080/13833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I647d9c3cf01807bd8ae9f8cf4091568ea6f1161c Gerrit-Change-Number: 13833 Gerrit-PatchSet: 16 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: honeyhexin
[kudu-CR] [tserver] Move cell and probe key size checking to prepare phase
Yingchun Lai has posted comments on this change. (
http://gerrit.cloudera.org:8080/13470 )
Change subject: [tserver] Move cell and probe key size checking to prepare phase
..
Patch Set 6:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/13470/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13470/5//COMMIT_MSG@9
PS5, Line 9: inefficient
> Nit: inefficient
Done
http://gerrit.cloudera.org:8080/#/c/13470/5//COMMIT_MSG@12
PS5, Line 12: this lightweight checking t
> Nit: this lightweight checking
Done
http://gerrit.cloudera.org:8080/#/c/13470/5//COMMIT_MSG@13
PS5, Line 13: locking row
> Nit: locking row keys.
Done
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.h
File src/kudu/common/row_operations.h:
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.h@105
PS5, Line 105: Status ReadIssetBitmap(const uint8_t** bitmap);
: Status ReadNullBitmap
> Could also shorten this to YES and NO. Because it's an enum class, you have
Done
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.h@112
PS5, Line 112: // Same as above, but store result in 'dst'.
: Status ReadColumn(const ColumnSchema& col, uint8_t* dst,
Status* row_status);
: // Some column which is non-nullable has allocated a cell to
row data in
: // RowOperationsPBEncoder::Add, even if its data is useless
(i.e. set to
: // NULL), we have to consume data in order to properly
validate subsequent
: // columns and rows.
: Status ReadColumnAndDiscard(const ColumnSchema& col);
: bool HasNext() const;
> These are now complicated enough that they deserve documentation.
Done
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc
File src/kudu/common/row_operations.cc:
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc@235
PS5, Line 235: "value too large for column '$0' ($1 bytes, maximum is
$2 bytes)",
> Nit: should be aligned to (check_cell_size == ...
Done
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc@236
PS5, Line 236: col.name(), ptr_slice->size(),
FLAGS_max_cell_size_bytes));
> DCHECK is probably sufficient.
Done
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc@240
PS5, Line 240: }
: *slice = Slice(_->indirect_data()[offset_in_indirect],
ptr_slice->size());
: } else {
> "After one row's column size has been found to exceed the limit and has bee
Done
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc@256
PS5, Line 256: } else {
> DCHECK is probably good enough here too.
Done
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc@499
PS5, Line 499: // update to perform.
> IIUC, we don't check cell sizes here because the primary key is immutable,
Yes, otherwise we are not able to update or delete an exist, cell size exceed
row.
http://gerrit.cloudera.org:8080/#/c/13470/5/src/kudu/common/row_operations.cc@605
PS5, Line 605:
RETURN_NOT_OK(op->split_row->Set(static_cast(tablet_col_idx), data));
> Why don't we check cell sizes here?
Not check, because it's a split key, which will not stored in database.
--
To view, visit http://gerrit.cloudera.org:8080/13470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c
Gerrit-Change-Number: 13470
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Mon, 29 Jul 2019 06:40:16 +
Gerrit-HasComments: Yes
[kudu-CR] [tserver] Move cell and probe key size checking to prepare phase
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13470 to look at the new patch set (#6). Change subject: [tserver] Move cell and probe key size checking to prepare phase .. [tserver] Move cell and probe key size checking to prepare phase It's inefficient to check cell and probe key size in apply phase, because we have to lock row key and construct new Slices for every binary type of columns. This patch move this lightweight checking to prepare phase when decoding values from protobuf and before locking row keys. Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c --- M src/kudu/clock/clock.h M src/kudu/common/row_operations-test.cc M src/kudu/common/row_operations.cc M src/kudu/common/row_operations.h M src/kudu/tablet/row_op.cc M src/kudu/tablet/row_op.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_bootstrap.cc 9 files changed, 239 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/13470/6 -- To view, visit http://gerrit.cloudera.org:8080/13470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id3e672272bb1dcf2d0ac1d96ee8a1a2d1489774c Gerrit-Change-Number: 13470 Gerrit-PatchSet: 6 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
