[kudu-CR] [catalog manager] re-replication warning message update
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/4 ) Change subject: [catalog_manager] re-replication warning message update .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/4/1/src/kudu/master/catalog_manager.cc@190 PS1, Line 190: not counting in the table's replication factor > As I understand from the code around line 1467, the value of this flag does After some consideration, I decided to keep that separate and remove from this patch. I think if we need to fix the description for this flag or somehow update the behavior, we can do that in a separate changelist. -- To view, visit http://gerrit.cloudera.org:8080/4 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I608ce179548ac25d660d713e7ce1d0e067dd4e02 Gerrit-Change-Number: 4 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 03 Aug 2018 04:57:13 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] re-replication warning message update
Hello Will Berkeley, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4 to look at the new patch set (#2). Change subject: [catalog_manager] re-replication warning message update .. [catalog_manager] re-replication warning message update Updated the warning message upon creating a table with replication factor F when only (F - 1) tablet servers are alive. Do not mention unsetting --raft_prepare_replacement_before_eviction since the 3-4-3 replica management scheme allows to spawn a new replica in-place in case of unrecoverable failure such as IO error and falling behind WAL's ancient history mark. So, the 3-2-3 scheme is no better in that regard and would not be able to re-replicate in case of a tablet server failure either. Also, the 3-2-3 scheme is deprecated at this point. I also did few other petty changes around the warning message. There are no functional changes in this patch. Change-Id: I608ce179548ac25d660d713e7ce1d0e067dd4e02 --- M src/kudu/master/catalog_manager.cc 1 file changed, 9 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/4/2 -- To view, visit http://gerrit.cloudera.org:8080/4 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I608ce179548ac25d660d713e7ce1d0e067dd4e02 Gerrit-Change-Number: 4 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2525: KuduTableInputFormat may halt before exhausting scan
zhangqianqiong has posted comments on this change. ( http://gerrit.cloudera.org:8080/1 ) Change subject: KUDU-2525: KuduTableInputFormat may halt before exhausting scan .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/1/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/1/1//COMMIT_MSG@7 PS1, Line 7: KUDU-2525: KuduTableInputFormat may halt before exhausting scan > Rewrite as: Done http://gerrit.cloudera.org:8080/#/c/1/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java File java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java: http://gerrit.cloudera.org:8080/#/c/1/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@66 PS1, Line 66: * This input format generates one split per tablet and the only location for each split is that > Given that there's one split per tablet, how does this bug manifest? I thou I can be sure that there is not an empty tablet in the table. scanner.nextRows() sometimes returned an empty RowResultIterator, and sometimes returned a normal RowResultIterator. In other words, the results is random. So, I guess that the scan is too time-consuming to timeout and returned the empty RowResultIerator http://gerrit.cloudera.org:8080/#/c/1/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@448 PS1, Line 448: // TODO(qqzhang) scanner.nextRows() sometimes returns an empty RowResultIterator, > Typically we mark things like this with TODO(...); what's left to do here? Done -- To view, visit http://gerrit.cloudera.org:8080/1 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbfdd2efbd281e4d849917664b33e183e180bafd Gerrit-Change-Number: 1 Gerrit-PatchSet: 2 Gerrit-Owner: zhangqianqiong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: zhangqianqiong Gerrit-Comment-Date: Fri, 03 Aug 2018 02:45:11 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2525: KuduTableInputFormat may halt before exhausting scan
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/1 to look at the new patch set (#2). Change subject: KUDU-2525: KuduTableInputFormat may halt before exhausting scan .. KUDU-2525: KuduTableInputFormat may halt before exhausting scan Change-Id: Ifbfdd2efbd281e4d849917664b33e183e180bafd --- M java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java 1 file changed, 6 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/1/2 -- To view, visit http://gerrit.cloudera.org:8080/1 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifbfdd2efbd281e4d849917664b33e183e180bafd Gerrit-Change-Number: 1 Gerrit-PatchSet: 2 Gerrit-Owner: zhangqianqiong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [catalog manager] re-replication warning message update
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/4 ) Change subject: [catalog_manager] re-replication warning message update .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/4/1/src/kudu/master/catalog_manager.cc@190 PS1, Line 190: not counting in the table's replication factor > what does this mean? As I understand from the code around line 1467, the value of this flag does not take into account replication factor. So, when creating a table of RF=3 with 60 partitions in a cluster with 3 tablet servers, the total number of replicas per tserver will be 60, but IIUC the code at line 1467 counts it as it were only one replica per tablet. -- To view, visit http://gerrit.cloudera.org:8080/4 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I608ce179548ac25d660d713e7ce1d0e067dd4e02 Gerrit-Change-Number: 4 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 03 Aug 2018 00:40:10 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] re-replication warning message update
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/4 ) Change subject: [catalog_manager] re-replication warning message update .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/4/1/src/kudu/master/catalog_manager.cc@190 PS1, Line 190: not counting in the table's replication factor what does this mean? -- To view, visit http://gerrit.cloudera.org:8080/4 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I608ce179548ac25d660d713e7ce1d0e067dd4e02 Gerrit-Change-Number: 4 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Fri, 03 Aug 2018 00:30:49 + Gerrit-HasComments: Yes
[kudu-CR] memrowset: support iteration with is deleted virtual column
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10990 ) Change subject: memrowset: support iteration with is_deleted virtual column .. Patch Set 9: Code-Review+1 lgtm aside from Dan's comment -- To view, visit http://gerrit.cloudera.org:8080/10990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6b053f5a3696eb9d7c26b8e3d96752f4f87bcd8 Gerrit-Change-Number: 10990 Gerrit-PatchSet: 9 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Aug 2018 00:27:27 + Gerrit-HasComments: No
[kudu-CR] schema: add is deleted virtual column
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 13 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Aug 2018 00:16:38 + Gerrit-HasComments: No
[kudu-CR] memrowset: support iteration with is deleted virtual column
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10990 ) Change subject: memrowset: support iteration with is_deleted virtual column .. Patch Set 9: (1 comment) > Patch Set 9: > > (1 comment) http://gerrit.cloudera.org:8080/#/c/10990/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10990/9//COMMIT_MSG@15 PS9, Line 15: We could require one or the other, but that : pokes a hole in the abstraction of a virtual column as just a data type, : which seems undesirable. FWIW I think this would be fine, the non-nullable PK restrictions are kind of similar. -- To view, visit http://gerrit.cloudera.org:8080/10990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6b053f5a3696eb9d7c26b8e3d96752f4f87bcd8 Gerrit-Change-Number: 10990 Gerrit-PatchSet: 9 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Aug 2018 00:06:02 + Gerrit-HasComments: Yes
[kudu-CR] memrowset: support iteration with is deleted virtual column
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10990 ) Change subject: memrowset: support iteration with is_deleted virtual column .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/10990/9/src/kudu/tablet/memrowset.cc File src/kudu/tablet/memrowset.cc: http://gerrit.cloudera.org:8080/#/c/10990/9/src/kudu/tablet/memrowset.cc@408 PS9, Line 408: projection_vc_is_deleted_idx_ = i; Is this missing a break? Otherwise it's the last instance. -- To view, visit http://gerrit.cloudera.org:8080/10990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6b053f5a3696eb9d7c26b8e3d96752f4f87bcd8 Gerrit-Change-Number: 10990 Gerrit-PatchSet: 9 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Aug 2018 00:04:53 + Gerrit-HasComments: Yes
[kudu-CR] schema: add is deleted virtual column
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 13: Code-Review+1 LGTM -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 13 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 23:24:52 + Gerrit-HasComments: No
[kudu-CR] [no review] Base to unblock threads of location awareness work
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/2 ) Change subject: [no review] Base to unblock threads of location awareness work .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/2/1/src/kudu/master/ts_descriptor.h File src/kudu/master/ts_descriptor.h: http://gerrit.cloudera.org:8080/#/c/2/1/src/kudu/master/ts_descriptor.h@123 PS1, Line 123: const boost::optional& This should return a copy since the location can change if the tablet server re-registers. -- To view, visit http://gerrit.cloudera.org:8080/2 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I38efb0acd94ed00b355c106875b9bb6c57ae2915 Gerrit-Change-Number: 2 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 02 Aug 2018 23:14:16 + Gerrit-HasComments: Yes
[kudu-CR] [tools] run rebalancer during 'election storm'
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11107 ) Change subject: [tools] run rebalancer during 'election storm' .. [tools] run rebalancer during 'election storm' Added an integration test to run the rebalancer tool in an 'election storm' environment. The rebalancer should run with no issues unless a tablet server is reported as unavailable. I ran the test 250 times in each build configuration and didn't spot any failures. Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 Reviewed-on: http://gerrit.cloudera.org:8080/11107 Tested-by: Kudu Jenkins Reviewed-by: Will Berkeley --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 155 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 Gerrit-Change-Number: 11107 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] [catalog manager] re-replication warning message update
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/4 Change subject: [catalog_manager] re-replication warning message update .. [catalog_manager] re-replication warning message update Updated the warning message upon creating a table with replication factor F when only (F - 1) tablet servers are alive. Do not mention unsetting --raft_prepare_replacement_before_eviction since the 3-4-3 replica management scheme allows to spawn a new replica in-place in case of unrecoverable failure such as IO error and falling behind WAL's ancient history mark. So, the 3-2-3 scheme is no better in that regard and would not be able to re-replicate in case of a tablet server failure either. Also, the 3-2-3 scheme is deprecated at this point. I also updated the description for the --max_create_tablets_per_ts flag to clarify on the replication factor not being taken into account and did other petty changes around the warning message. There are no functional changes in this patch. Change-Id: I608ce179548ac25d660d713e7ce1d0e067dd4e02 --- M src/kudu/master/catalog_manager.cc 1 file changed, 12 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/4/1 -- To view, visit http://gerrit.cloudera.org:8080/4 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I608ce179548ac25d660d713e7ce1d0e067dd4e02 Gerrit-Change-Number: 4 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [tools] run rebalancer during 'election storm'
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 ) Change subject: [tools] run rebalancer during 'election storm' .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 Gerrit-Change-Number: 11107 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 02 Aug 2018 23:05:30 + Gerrit-HasComments: No
[kudu-CR] [tools] run rebalancer during 'election storm'
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 ) Change subject: [tools] run rebalancer during 'election storm' .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2093 PS1, Line 2093: auto max_sleep_ms = 2.0; > What do you mean by "module expression"? Oh, that was a typo. I meant 'modulo' :) And then there is std::min(x * 1.1, 2000.0) step. I added corresponding comment into the code. -- To view, visit http://gerrit.cloudera.org:8080/11107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 Gerrit-Change-Number: 11107 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 02 Aug 2018 21:50:13 + Gerrit-HasComments: Yes
[kudu-CR] [tools] run rebalancer during 'election storm'
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 ) Change subject: [tools] run rebalancer during 'election storm' .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2086 PS1, Line 2086: #if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER) : const auto timeout = MonoDelta::FromSeconds(5); : #else : const auto timeout = MonoDelta::FromSeconds(10); > The timeout here is a time-to-run for the stormy elector thread as well. S (thumbsup) http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2093 PS1, Line 2093: auto max_sleep_ms = 2.0; > It was the lowest possible to be used in module expression. What do you mean by "module expression"? http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2109 PS1, Line 2109: for (const auto& tablet : tablets) { > Yes, that's what I want -- no matter how, but initiate as much elections as (thumbsup) http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2164 PS1, Line 2164: usually happens because GetConsensusState requests are dropped due to : // backpressure > We should, and as soon as it's done in a separate changelist, I'm thinking (thumbsup) -- To view, visit http://gerrit.cloudera.org:8080/11107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 Gerrit-Change-Number: 11107 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 02 Aug 2018 21:28:35 + Gerrit-HasComments: Yes
[kudu-CR] [tools] run rebalancer during 'election storm'
Hello Will Berkeley, Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11107 to look at the new patch set (#2). Change subject: [tools] run rebalancer during 'election storm' .. [tools] run rebalancer during 'election storm' Added an integration test to run the rebalancer tool in an 'election storm' environment. The rebalancer should run with no issues unless a tablet server is reported as unavailable. I ran the test 250 times in each build configuration and didn't spot any failures. Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 --- M src/kudu/tools/kudu-admin-test.cc 1 file changed, 155 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/11107/2 -- To view, visit http://gerrit.cloudera.org:8080/11107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 Gerrit-Change-Number: 11107 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] [tools] run rebalancer during 'election storm'
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 ) Change subject: [tools] run rebalancer during 'election storm' .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2072 PS1, Line 2072: : RebalancingTest() { > warning: initializer for base class 'kudu::tools::RebalancingTest' is redun Done http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2086 PS1, Line 2086: #if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER) : const auto timeout = MonoDelta::FromSeconds(5); : #else : const auto timeout = MonoDelta::FromSeconds(10); > I think you swapped the timeouts here because the sanitizer builds should h The timeout here is a time-to-run for the stormy elector thread as well. So no, I didn't swapped those. Making longer timeout for workload in case of TSAN/ASAN is not needed -- the code is written to tolerate timeouts, if any: having everything generated written into the tables is not required. http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2093 PS1, Line 2093: auto max_sleep_ms = 2.0; > Can you explain where this number comes from? Obviously it's super low comp It was the lowest possible to be used in module expression. http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2109 PS1, Line 2109: for (const auto& tablet : tablets) { > With RF=3, running over each TS and replica will cause 3 elections to be ca Yes, that's what I want -- no matter how, but initiate as much elections as possible, but implemented with easy to follow code. http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2164 PS1, Line 2164: usually happens because GetConsensusState requests are dropped due to : // backpressure > As you mentioned to me in person, we should fix ksck to be resilient to thi We should, and as soon as it's done in a separate changelist, I'm thinking to update this part. -- To view, visit http://gerrit.cloudera.org:8080/11107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 Gerrit-Change-Number: 11107 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 02 Aug 2018 20:53:26 + Gerrit-HasComments: Yes
[kudu-CR] schema: add is deleted virtual column
Hello Tidy Bot, Mike Percy, Alexey Serbin, Dan Burkert, Kudu Jenkins, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10968 to look at the new patch set (#13). Change subject: schema: add is_deleted virtual column .. schema: add is_deleted virtual column This patch introduces a very basic concept of a "virtual column". Virtual columns borrow from other databases in that they are columns that, rather than being backed by physical data, are instead backed by Kudu itself. They may not be part of a schema during table creation/alteration, but may be added to projections during a scan. Kudu's virtual columns are defined as logical data types. As data types are not user-defined, there's no danger of a "collision" between a virtual column and a physical column as there would be if a virtual column occupied a well-defined name. A Kudu subsystem on the scan path that wishes to interact with a virtual column needs to first figure out if the projection includes it. When projected, the virtual column's data will be either some default or null (depending on exactly how it was defined in the projection); it's the subsystem's responsibility to fill in something meaningful afterwards. Beyond the basic definition, this patch introduces an IS_DELETED virtual column derived from BOOL. IS_DELETED will be used to support incremental backups by describing whether a row was deleted between two timestamps. Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e --- M src/kudu/common/common.proto M src/kudu/common/schema.cc M src/kudu/common/types.cc M src/kudu/common/types.h M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc 6 files changed, 119 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/10968/13 -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 13 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] memrowset: support iteration with is deleted virtual column
Hello Mike Percy, Kudu Jenkins, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10990 to look at the new patch set (#9). Change subject: memrowset: support iteration with is_deleted virtual column .. memrowset: support iteration with is_deleted virtual column This patch rounds out the MemRowSet changes for incremental backups. Taken together, it is now possible to iterate on a specific time range and to learn which rows were deleted during that time range. Data type based virtual columns show a wart here: the iterator doesn't know whether IS_DELETED was defined as nullable or with a read default value, and it must be prepared for either. We could require one or the other, but that pokes a hole in the abstraction of a virtual column as just a data type, which seems undesirable. Change-Id: Ic6b053f5a3696eb9d7c26b8e3d96752f4f87bcd8 --- M src/kudu/tablet/memrowset-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h 3 files changed, 135 insertions(+), 49 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/10990/9 -- To view, visit http://gerrit.cloudera.org:8080/10990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6b053f5a3696eb9d7c26b8e3d96752f4f87bcd8 Gerrit-Change-Number: 10990 Gerrit-PatchSet: 9 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] schema: add is deleted virtual column
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 12: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 12 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 20:21:14 + Gerrit-HasComments: No
[kudu-CR] [no review] Base to unblock threads of location awareness work
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/2 Change subject: [no review] Base to unblock threads of location awareness work .. [no review] Base to unblock threads of location awareness work Change-Id: I38efb0acd94ed00b355c106875b9bb6c57ae2915 --- M src/kudu/master/ts_descriptor.h 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/2/1 -- To view, visit http://gerrit.cloudera.org:8080/2 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I38efb0acd94ed00b355c106875b9bb6c57ae2915 Gerrit-Change-Number: 2 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] schema: add is deleted virtual column
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h File src/kudu/common/types.h: http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@158 PS11, Line 158: static bool IsVirtual() { : return false; : } > Is that possible? I thought the DataTypeTraits template was intentionally e Woops, I see. That would be possible if not those exact type-coded references like TypeTraitsClass::min_value() and alike. But it seems that was designed to be like this. http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@673 PS11, Line 673: case IS_DELETED: > As another point of evidence, Clang doesn't warn for statement-less cases, That makes sense. I thought the code style just mandates having those FALLTHROUGH_INTENDED. -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 11 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 20:08:45 + Gerrit-HasComments: Yes
[kudu-CR] schema: add is deleted virtual column
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 12: Code-Review+1 Overall lgtm -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 12 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 19:04:01 + Gerrit-HasComments: No
[kudu-CR] schema: add is deleted virtual column
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h File src/kudu/common/types.h: http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@673 PS11, Line 673: case IS_DELETED: > As another point of evidence, Clang doesn't warn for statement-less cases, This is an interesting perspective Dan, something stylistic I have thought about in the past without a satisfying conclusion and I think your argument makes a lot of sense. -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 12 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 18:58:28 + Gerrit-HasComments: Yes
[kudu-CR] schema: add is deleted virtual column
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h File src/kudu/common/types.h: http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@673 PS11, Line 673: case IS_DELETED: > The downside of adding FALLTHROUGH_INTENDED is that it's just obfuscating n As another point of evidence, Clang doesn't warn for statement-less cases, and clang::fallthrough is why this is a macro and not just a comment convention: https://clang.llvm.org/docs/AttributeReference.html#fallthrough-clang-fallthrough -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 12 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 18:55:39 + Gerrit-HasComments: Yes
[kudu-CR] schema: add is deleted virtual column
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h File src/kudu/common/types.h: http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@673 PS11, Line 673: case IS_DELETED: > In my opinion this isn't an appropriate use for FALLTHRUGH_INTENDED. It sh The downside of adding FALLTHROUGH_INTENDED is that it's just obfuscating noise; no one's taken by surprise that statement-less cases fall through. -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 12 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 18:52:37 + Gerrit-HasComments: Yes
[kudu-CR] schema: add is deleted virtual column
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h File src/kudu/common/types.h: http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@673 PS11, Line 673: case IS_DELETED: > nit: add FALLTHROUGH_INTENDED ? In my opinion this isn't an appropriate use for FALLTHRUGH_INTENDED. It should only be used when a case contains an statement, and then after evaluating that statement falls into another case. The docs on FALLTHROUGH_INTENDED show this: // switch (x) { //case 40: //case 41: // if (truth_is_out_there) { //++x; //FALLTHROUGH_INTENDED; // Use instead of/along with annotations in // // comments. // } else { //return x; // } //case 42: // ... Notice how there is no FALLTHROUGH_INTENDED annotation for the (statement-less) case 40. -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 12 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 18:50:48 + Gerrit-HasComments: Yes
[kudu-CR] schema: add is deleted virtual column
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 12: (4 comments) http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h File src/kudu/common/types.h: http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@158 PS11, Line 158: static bool IsVirtual() { : return false; : } > nit: maybe, move this into the DataTypeTraits template itself and have cust Is that possible? I thought the DataTypeTraits template was intentionally empty; if you look at some of the other specializations, there's what appears to be intentional repetition of implementations (i.e. min_value() and max_value()). I just tried it out: I added an IsVirtual static function to the DataTypeTraits template, removed it from the specializations, but kept it in DerivedTypeTraits and DataTypeTraits. I got the following compilation failure: FAILED: src/kudu/common/CMakeFiles/kudu_common.dir/types.cc.o /usr/lib/ccache/c++ -DHAVE_LIB_VMEM=1 -DKUDU_HEADERS_NO_STUBS=1 -DKUDU_HEADERS_USE_RICH_SLICE=1 -DKUDU_HEADERS_USE_SHORT_STATUS_MACROS=1 -DKUDU_STATIC_DEFINE -DTCMALLOC_ENABLED -D__STDC_FORMAT_MACROS -Dkudu_common_EXPORTS -Isrc -I../../src -isystem ../../thirdparty/installed/common/include -isystem ../../thirdparty/installed/uninstrumented/include -msse4.2 -Wall -Wno-sign-compare -Wno-comment -Wno-deprecated -pthread -fno-strict-aliasing -DBOOST_DATE_TIME_POSIX_TIME_STD_CONFIG -ggdb -std=c++11 -fuse-ld=gold -fsized-deallocation -g -fPIC -fPIC -MD -MT src/kudu/common/CMakeFiles/kudu_common.dir/types.cc.o -MF src/kudu/common/CMakeFiles/kudu_common.dir/types.cc.o.d -o src/kudu/common/CMakeFiles/kudu_common.dir/types.cc.o -c ../../src/kudu/common/types.cc ../../src/kudu/common/types.cc: In instantiation of ‘kudu::TypeInfo::TypeInfo(TypeTraitsClass) [with Type = kudu::TypeTraits<(kudu::DataType)0>]’: ../../src/kudu/common/types.cc:96:49: required from ‘void kudu::TypeInfoResolver::AddMapping() [with kudu::DataType type = (kudu::DataType)0]’ ../../src/kudu/common/types.cc:73:23: required from here ../../src/kudu/common/types.cc:40:43: error: ‘IsVirtual’ is not a member of ‘kudu::TypeTraits<(kudu::DataType)0>’ is_virtual_(TypeTraitsClass::IsVirtual()), ~~^~ ../../src/kudu/common/types.cc: In instantiation of ‘kudu::TypeInfo::TypeInfo(TypeTraitsClass) [with Type = kudu::TypeTraits<(kudu::DataType)1>]’: ../../src/kudu/common/types.cc:96:49: required from ‘void kudu::TypeInfoResolver::AddMapping() [with kudu::DataType type = (kudu::DataType)1]’ ../../src/kudu/common/types.cc:74:22: required from here ../../src/kudu/common/types.cc:40:43: error: ‘IsVirtual’ is not a member of ‘kudu::TypeTraits<(kudu::DataType)1>’ ../../src/kudu/common/types.cc: In instantiation of ‘kudu::TypeInfo::TypeInfo(TypeTraitsClass) [with Type = kudu::TypeTraits<(kudu::DataType)2>]’: ../../src/kudu/common/types.cc:96:49: required from ‘void kudu::TypeInfoResolver::AddMapping() [with kudu::DataType type = (kudu::DataType)2]’ ../../src/kudu/common/types.cc:75:24: required from here ../../src/kudu/common/types.cc:40:43: error: ‘IsVirtual’ is not a member of ‘kudu::TypeTraits<(kudu::DataType)2>’ ../../src/kudu/common/types.cc: In instantiation of ‘kudu::TypeInfo::TypeInfo(TypeTraitsClass) [with Type = kudu::TypeTraits<(kudu::DataType)3>]’: ../../src/kudu/common/types.cc:96:49: required from ‘void kudu::TypeInfoResolver::AddMapping() [with kudu::DataType type = (kudu::DataType)3]’ ../../src/kudu/common/types.cc:76:23: required from here ../../src/kudu/common/types.cc:40:43: error: ‘IsVirtual’ is not a member of ‘kudu::TypeTraits<(kudu::DataType)3>’ ../../src/kudu/common/types.cc: In instantiation of ‘kudu::TypeInfo::TypeInfo(TypeTraitsClass) [with Type = kudu::TypeTraits<(kudu::DataType)4>]’: ../../src/kudu/common/types.cc:96:49: required from ‘void kudu::TypeInfoResolver::AddMapping() [with kudu::DataType type = (kudu::DataType)4]’ ../../src/kudu/common/types.cc:77:24: required from here ../../src/kudu/common/types.cc:40:43: error: ‘IsVirtual’ is not a member of ‘kudu::TypeTraits<(kudu::DataType)4>’ ../../src/kudu/common/types.cc: In instantiation of ‘kudu::TypeInfo::TypeInfo(TypeTraitsClass) [with Type = kudu::TypeTraits<(kudu::DataType)5>]’: ../../src/kudu/common/types.cc:96:49: required from ‘void kudu::TypeInfoResolver::AddMapping() [with kudu::DataType type = (kudu::DataType)5]’ ../../src/kudu/common/types.cc:78:23: required from here ../../src/kudu/common/types.cc:40:43: error: ‘IsVirtual’ is not a member of ‘kudu::TypeTraits<(kudu::DataType)5>’ ../../src/kudu/common/types.cc: In instantiation of ‘kudu::TypeInfo::TypeInfo(TypeTraitsClass) [with Type = kudu::TypeTraits<(kudu::DataType)6>]’:
[kudu-CR] schema: add is deleted virtual column
Hello Tidy Bot, Mike Percy, Alexey Serbin, Dan Burkert, Kudu Jenkins, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10968 to look at the new patch set (#12). Change subject: schema: add is_deleted virtual column .. schema: add is_deleted virtual column This patch introduces a very basic concept of a "virtual column". Virtual columns borrow from other databases in that they are columns that, rather than being backed by physical data, are instead backed by Kudu itself. They may not be part of a schema during table creation/alteration, but may be added to projections during a scan. Kudu's virtual columns are defined as logical data types. As data types are not user-defined, there's no danger of a "collision" between a virtual column and a physical column as there would be if a virtual column occupied a well-defined name. A Kudu subsystem on the scan path that wishes to interact with a virtual column needs to first figure out if the projection includes it. When projected, the virtual column's data will be either some default or null (depending on exactly how it was defined in the projection); it's the subsystem's responsibility to fill in something meaningful afterwards. Beyond the basic definition, this patch introduces an IS_DELETED virtual column derived from BOOL. IS_DELETED will be used to support incremental backups by describing whether a row was deleted between two timestamps. Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e --- M src/kudu/common/common.proto M src/kudu/common/schema.cc M src/kudu/common/types.cc M src/kudu/common/types.h M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc 6 files changed, 145 insertions(+), 47 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/10968/12 -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 12 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Kudu-1291 Efficiently support predicates on non-prefix key components
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10983 ) Change subject: Kudu-1291 Efficiently support predicates on non-prefix key components .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10983/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10983/5//COMMIT_MSG@7 PS5, Line 7: Kudu-1291 nit: s/Kudu-1291/KUDU-1291/ Also preferably add a period or a colon after the JIRA number, like KUDU-1291. Efficiently blah blah -- To view, visit http://gerrit.cloudera.org:8080/10983 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f Gerrit-Change-Number: 10983 Gerrit-PatchSet: 5 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Thu, 02 Aug 2018 18:33:23 + Gerrit-HasComments: Yes
[kudu-CR] Kudu-1291 Efficiently support predicates on non-prefix key components
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10983 ) Change subject: Kudu-1291 Efficiently support predicates on non-prefix key components .. Patch Set 5: > Ah, I see -- thank you for letting me know. I thought since the WIP tag was > removed it's more or less ready. No worries, it's actually pretty close, would love to get your feedback on the algorithm -- To view, visit http://gerrit.cloudera.org:8080/10983 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f Gerrit-Change-Number: 10983 Gerrit-PatchSet: 5 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Thu, 02 Aug 2018 18:32:02 + Gerrit-HasComments: No
[kudu-CR] [tools] run rebalancer during 'election storm'
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11107 ) Change subject: [tools] run rebalancer during 'election storm' .. Patch Set 1: (4 comments) I'm guessing you already did, but if you haven't can you run this a bunch of times with dist-test to check it's not flaky, and add some stats about that to the commit msg? http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2086 PS1, Line 2086: #if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER) : const auto timeout = MonoDelta::FromSeconds(5); : #else : const auto timeout = MonoDelta::FromSeconds(10); I think you swapped the timeouts here because the sanitizer builds should have longer timeouts. http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2093 PS1, Line 2093: auto max_sleep_ms = 2.0; Can you explain where this number comes from? Obviously it's super low compared to the expected time to an election start from a leader stepdown in an idle cluster of about 1.25 seconds, but the Raft heartbeat times are changed for the tests. A couple sentences explaining the initial setting and backoff would be good. http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2109 PS1, Line 2109: for (const auto& tablet : tablets) { With RF=3, running over each TS and replica will cause 3 elections to be called on each tablet. Is that what you want? Or do you want one per tablet per cycle through all the tablets? http://gerrit.cloudera.org:8080/#/c/11107/1/src/kudu/tools/kudu-admin-test.cc@2164 PS1, Line 2164: usually happens because GetConsensusState requests are dropped due to : // backpressure As you mentioned to me in person, we should fix ksck to be resilient to this up to some timeout. -- To view, visit http://gerrit.cloudera.org:8080/11107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic98684dbe55049bbc411513faa0b6bbaef20f434 Gerrit-Change-Number: 11107 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 02 Aug 2018 18:15:39 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2525: fix a bug for kudu: scanner return the zero rows
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/1 ) Change subject: KUDU-2525: fix a bug for kudu: scanner return the zero rows .. Patch Set 1: (3 comments) Could you reproduce the bug in a kudu-mapreduce unit test? http://gerrit.cloudera.org:8080/#/c/1/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/1/1//COMMIT_MSG@7 PS1, Line 7: KUDU-2525: fix a bug for kudu: scanner return the zero rows Rewrite as: KUDU-2525: KuduTableInputFormat may halt before exhausting scan http://gerrit.cloudera.org:8080/#/c/1/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java File java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java: http://gerrit.cloudera.org:8080/#/c/1/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@66 PS1, Line 66: * This input format generates one split per tablet and the only location for each split is that Given that there's one split per tablet, how does this bug manifest? I thought the only time scanner.nextRows() returned an empty RowResultIterator was when the scan ended on one tablet and started up on the next, and the next tablet didn't have any rows. http://gerrit.cloudera.org:8080/#/c/1/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@448 PS1, Line 448: // FIXME(qqzhang) the loop is only for the responded empty rows Typically we mark things like this with TODO(...); what's left to do here? Could you instead modify this comment to make it more instructive? You can explain how scanner.nextRows() is allowed to return an empty RowResultIterator, but until scanner.hasMoreRows() returns false, we need to continue iterating on the scanner. -- To view, visit http://gerrit.cloudera.org:8080/1 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifbfdd2efbd281e4d849917664b33e183e180bafd Gerrit-Change-Number: 1 Gerrit-PatchSet: 1 Gerrit-Owner: zhangqianqiong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 02 Aug 2018 18:07:14 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2459: add placeholder names to some CREATE TABLE statements
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10656 ) Change subject: KUDU-2459: add placeholder names to some CREATE TABLE statements .. KUDU-2459: add placeholder names to some CREATE TABLE statements Under the Tables section of Kudu Web UI, for a selected table, the table metrics display a CREATE TABLE statement that can be run to make Impala cognizant of that table. However, in generation of this statement, the tablename tries to match the original Kudu tablename which may not always be acceptable as a tablename for Impala. For example, Kudu accepts dot in tablename, Impala does not. The CREATE TABLE statement thus throws an invalid tablename error in Impala. We considered trying to derive an Impala-conforming name from the Kudu- supplied tablename but that could result in surprising/unintuitive results. We also want to leverage the current functionality of auto- generated statement where the name is valid. Therefore with this update, invalid tablenames have been identified and updated by a placeholder tablename for end user to replace with a valid tablename. The rules to check Impala conformity are below -- - The minimum length of an identifier is 1 character. - The maximum length of an identifier is currently 128 characters. - An identifier must start with an alphabetic character. The remainder can contain any combination of alphanumeric characters and underscores. Quoting the identifier with backticks has no effect on the allowed characters in the name. - An identifier can contain only ASCII characters. Change-Id: If9e5242318589452a0507d3973b9be3e6a186a69 Reviewed-on: http://gerrit.cloudera.org:8080/10656 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/master/master_path_handlers.cc M www/table.mustache 2 files changed, 28 insertions(+), 1 deletion(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If9e5242318589452a0507d3973b9be3e6a186a69 Gerrit-Change-Number: 10656 Gerrit-PatchSet: 17 Gerrit-Owner: Shriya Gupta Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Shriya Gupta Gerrit-Reviewer: Thomas Marshall
[kudu-CR] schema: add is deleted virtual column
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10968 ) Change subject: schema: add is_deleted virtual column .. Patch Set 11: (4 comments) http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h File src/kudu/common/types.h: http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@158 PS11, Line 158: static bool IsVirtual() { : return false; : } nit: maybe, move this into the DataTypeTraits template itself and have custom specialization just for IS_DELETED? http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@673 PS11, Line 673: case IS_DELETED: nit: add FALLTHROUGH_INTENDED ? http://gerrit.cloudera.org:8080/#/c/10968/11/src/kudu/common/types.h@763 PS11, Line 763: case IS_DELETED: nit: add FALLTHROUGH_INTENDED ? http://gerrit.cloudera.org:8080/#/c/10968/10/src/kudu/common/types.h File src/kudu/common/types.h: http://gerrit.cloudera.org:8080/#/c/10968/10/src/kudu/common/types.h@625 PS10, Line 625: return DataTypeTraits::name(); > It was at first, but that led to a row dump that looked like this: But if it's name is 'bool', then how to distinguish between a column of virtual type and regular bool in a row dump? Or we don't need to differentiate between those in row dumps? >From the other side, what if the new type was named 'VIRTULAL_BOOL' or >'VBOOL' instead of 'IS_DELETED'? -- To view, visit http://gerrit.cloudera.org:8080/10968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56eb1d44ba8bfbd76d8bb794b8076b695782939e Gerrit-Change-Number: 10968 Gerrit-PatchSet: 11 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 14:30:54 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2525: fix a bug for kudu: scanner return the zero rows
zhangqianqiong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/1 Change subject: KUDU-2525: fix a bug for kudu: scanner return the zero rows .. KUDU-2525: fix a bug for kudu: scanner return the zero rows Change-Id: Ifbfdd2efbd281e4d849917664b33e183e180bafd --- M java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/1/1 -- To view, visit http://gerrit.cloudera.org:8080/1 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifbfdd2efbd281e4d849917664b33e183e180bafd Gerrit-Change-Number: 1 Gerrit-PatchSet: 1 Gerrit-Owner: zhangqianqiong
[kudu-CR] Implement BloomFilter Predicate in server side.
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11100 to look at the new patch set (#2). Change subject: Implement BloomFilter Predicate in server side. .. Implement BloomFilter Predicate in server side. Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c --- M src/kudu/common/column_predicate-test.cc M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h M src/kudu/common/common.proto M src/kudu/common/key_util.cc M src/kudu/common/scan_spec.cc M src/kudu/common/wire_protocol-test.cc M src/kudu/common/wire_protocol.cc M src/kudu/tablet/cfile_set-test.cc 9 files changed, 988 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/11100/2 -- To view, visit http://gerrit.cloudera.org:8080/11100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I62c2de42667d0255d94e19db773240f7f9ee636c Gerrit-Change-Number: 11100 Gerrit-PatchSet: 2 Gerrit-Owner: ZhangYao Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon