[kudu-CR] tool: add cluster shell action
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/7853 ) Change subject: tool: add cluster shell action .. Patch Set 8: (11 comments) Just skimmed through. Will take a deeper look tomorrow. http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/kudu-tool-test.cc@1843 PS8, Line 1843: ASSERT_FALSE(s.ok()); : ASSERT_TRUE(s.IsInvalidArgument()); here and below: why is it necessary to verify for ASSERT_FALSE(s.ok()); if next line is about ASSERT_TRUE(s.IsInvlidArgument())? http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool.proto File src/kudu/tools/tool.proto: http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool.proto@60 PS8, Line 60: The generated ID of the cluster, unique to the control shell. maybe just: 'ID of the cluster to destroy.' ? http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool.proto@91 PS8, Line 91: required HostPortPB process_address How does it work for multiple addresses (corresponding to '--rpc_bind_addresses' flag)? Or we never want to have that test cluster bound to multiple addresses? http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool.proto@104 PS8, Line 104: HostPortPB Same question if multiple addresses are allowed: which address should be specified here -- the first one or any of the bound addresses? http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool_action_test.cc File src/kudu/tools/tool_action_test.cc: http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool_action_test.cc@20 PS8, Line 20: nit: please use instead. http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool_action_test.cc@64 PS8, Line 64: ; nit: remove extra semicolon http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool_action_test.cc@102 PS8, Line 102: InvalidArgument nit: maybe, NotFound is the better choice here? http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool_action_test.cc@133 PS8, Line 133: InvalidArgument ditto regarding NotFound http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool_action_test.cc@150 PS8, Line 150: FLAGS_serialization == "json" As I see, case-insensitive comparison (i.e. boost::iequals(v, "json")) is used in the validator. Maybe, it makes sense to compare case-insensitive comparison here as well? http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool_action_test.cc@153 PS8, Line 153: "pb", FLAGS_serialization ditto for case-insensitive comparison. http://gerrit.cloudera.org:8080/#/c/7853/8/src/kudu/tools/tool_action_test.cc@216 PS8, Line 216: opts.master_rpc_ports = { 11030, 11031, 11032 }; nit: add DCHECK_EQ(3, opts.num_masters) ? -- To view, visit http://gerrit.cloudera.org:8080/7853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e693921ef780dc4a06e536c6b7408f7f0b252f6 Gerrit-Change-Number: 7853 Gerrit-PatchSet: 8 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 03 Oct 2017 04:30:17 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1125: issue one catalog write per tablet report
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8090 ) Change subject: KUDU-1125: issue one catalog write per tablet report .. Patch Set 7: (9 comments) This is a big improvement, Adar. http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3106 PS6, Line 3106:"requestor", rpc->requestor_string(), > would be nice to also use TRACE_COUNTER_INCREMENT("reported_tablets", updat Todd, I'm not sure how your example can help measure latency. Did you mean to suggest using something like TRACE_COUNTER_SCOPE_LATENCY_US() ? http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3113 PS6, Line 3113: TODO(todd) per git blame http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3152 PS7, Line 3152: acqurie acquire http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3173 PS7, Line 3173: // reasonable to ignore it and wait for an operator fix the situation. The end of this comment only makes sense after going to the flag definition and seeing that it defaults to false. I wonder, if we are going to default it to false, if we shouldn't just remove it and this by-default-disabled logic altogether and simply emit the warning on L3181. When was the last time we enabled this? I actually thought the default behavior of the master was to delete unknown tablets. http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3236 PS7, Line 3236: report.has_consensus_state() I think this tertiary if condition needs to be: (report.has_consensus_state() && report.consensus_state().committed_config().has_opid_index()) because of the issue brought up in the comment on L3269. http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3291 PS7, Line 3291: Should it? Interesting question. I think it technically should, because the purpose of disregarding the non-member leaders is to wait until a leader is part of the committed config before publishing its existence to clients. OTOH I can't easily come up with a series of events where this would be a problem because to change the config there has to be an initial leader that coordinates the config change, and this logic is about waiting for that first leader when creating a new tablet from scratch for the first time. http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3319 PS7, Line 3319: config cstate; a config doesn't really have a term http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3320 PS7, Line 3320: config cstate http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3452 PS7, Line 3452: as in -- To view, visit http://gerrit.cloudera.org:8080/8090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6f5cf0e4b1cd1160b3b310d89c6dbf3dd62e43b Gerrit-Change-Number: 8090 Gerrit-PatchSet: 7 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 03 Oct 2017 02:55:41 + Gerrit-HasComments: Yes
[kudu-CR] WIP [consensus] adding/removing NON VOTER members
Hello Tidy Bot, Mike Percy, David Ribeiro Alves, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8138 to look at the new patch set (#6). Change subject: WIP [consensus] adding/removing NON_VOTER members .. WIP [consensus] adding/removing NON_VOTER members Added ability to add and remove NON_VOTER member replicas. Updated the kudu CLI accordingly. Also, added new integration test: * RaftConsensusITest.AddNonVoterReplica * RaftConsensusITest.AddThenRemoveNonVoterReplica * RaftConsensusITest.NonVoterReplicasDoNotVote WIP: Perhaps, I should continue gathering feedback on this patch and adding more tests. Change-Id: I2662d45ad9bb6a4bf325d4202c2ee619ffad02b7 --- M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/leader_election.cc M src/kudu/consensus/leader_election.h M src/kudu/consensus/metadata.proto M src/kudu/consensus/quorum_util-test.cc M src/kudu/consensus/quorum_util.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/raft_consensus-itest.cc 9 files changed, 488 insertions(+), 58 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/8138/6 -- To view, visit http://gerrit.cloudera.org:8080/8138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2662d45ad9bb6a4bf325d4202c2ee619ffad02b7 Gerrit-Change-Number: 8138 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Alexey Serbin has abandoned this change. ( http://gerrit.cloudera.org:8080/8133 ) Change subject: [tablet copy] end session before bootstrapping tablet .. Abandoned Abandoned in favor of http://gerrit.cloudera.org:8080/8197 -- To view, visit http://gerrit.cloudera.org:8080/8133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I20592007a40113d8409f121b226ba14e8300 Gerrit-Change-Number: 8133 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [tablet copy] comment on TabletCopyClient lifecycle
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8197 Change subject: [tablet copy] comment on TabletCopyClient lifecycle .. [tablet copy] comment on TabletCopyClient lifecycle Added an explanatory comment on the TabletCopyClient instance lifecycle. That was the result of an attempt to clean-up corresponding code: https://gerrit.cloudera.org/#/c/8133/ Change-Id: I7ae41f12fe75f05c84518c26b6875f34efae0172 --- M src/kudu/tserver/ts_tablet_manager.cc 1 file changed, 32 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/8197/1 -- To view, visit http://gerrit.cloudera.org:8080/8197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7ae41f12fe75f05c84518c26b6875f34efae0172 Gerrit-Change-Number: 8197 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-1125: issue one catalog write per tablet report
Hello Mike Percy, Dan Burkert, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8090 to look at the new patch set (#7). Change subject: KUDU-1125: issue one catalog write per tablet report .. KUDU-1125: issue one catalog write per tablet report This commit addresses a long-standing issue in the catalog manager where an independent catalog write is performed for each reported tablet. When the master is configured to fsync WAL writes, this can add a lot of load during election storms and when the master is restarted. To tackle this, I fully rewrote ProcessTabletReport and friends. I had higher hopes for the final product, but all of the dependent control flow complicates decomposition. Still, I managed to make some improvements. For example, all RPCs are now sent at the end in one go rather than piecemeal. I also rewrote all of the comments so that they can serve as a map to the function and to emphasize the actions performed by the corresponding code. Here are the actual semantic changes being made: - Table and tablet locks are now acquired en masse. For tablets, this is required for correctness; I've documented why I did the same for tables. - We no longer check for non-running tables. AFAICT this was a useless check because a non-running table must be a deleted table, and there's a check for that just before. - We no longer wake the BgTasks thread upon completion. This is because: 1. WakeIfHasPendingUpdates() was semi-broken; pending_updates_ is never set in ProcessTabletReport. 2. Regardless, there's no work for the BgTasks thread to do. Change-Id: Ie6f5cf0e4b1cd1160b3b310d89c6dbf3dd62e43b --- M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 2 files changed, 387 insertions(+), 402 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/8090/7 -- To view, visit http://gerrit.cloudera.org:8080/8090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie6f5cf0e4b1cd1160b3b310d89c6dbf3dd62e43b Gerrit-Change-Number: 8090 Gerrit-PatchSet: 7 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-1125: issue one catalog write per tablet report
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8090 ) Change subject: KUDU-1125: issue one catalog write per tablet report .. Patch Set 6: (10 comments) http://gerrit.cloudera.org:8080/#/c/8090/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8090/6//COMMIT_MSG@10 PS6, Line 10: When the : master is configured to fsync WAL writes, this can add a lot of load during : election storms and when the master is restarted. > any chance you did a before/after test here? eg start a small cluster with Haven't done any perf testing yet. I was planning to do it post-merge, but I'll tackle it now instead. I'll also try to experiment to see whether your estimate for max tablets is correct. It's tough because although it's pretty easy to trigger a full report (restart the master or a tserver), the underlying write performed by that report is likely to be sparse because 3b6ab64 will filter out all tablets that haven't actually changed. Meanwhile, events that do yield real changes (such as an election storm or the creation of a table) tend to be staggered over time. In short, it's hard to create a full report with nothing but real changes. http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3106 PS6, Line 3106:"num_tablets", full_report.updated_tablets_size()); > would be nice to also use TRACE_COUNTER_INCREMENT("reported_tablets", updat Done http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3122 PS6, Line 3122: unordered_map updates; > worth adding to the comment explaining ownership of these raw pointers Done http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3151 PS6, Line 3151: ReportedTabletUpdatesPB* update = full_report_update->add_tablets(); > to cut down on small allocations here, consider doing full_report_update->m Done http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3157 PS6, Line 3157: shared_lock l(lock_); > I think this would likely perform better if you can acquire this once for t I think I can do that without much refactoring, just need a new scope here. http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3227 PS6, Line 3227: bootstrapping > should this now say "copying"? Yeah. The original wording was "This prevents us from spuriously deleting replicas that have just been added to a pending config and are in the process of catching up to the log entry where they were added to the config." Dan asked me whether it should say "just been added to the committed config and are in the process of bootstrapping" and I misinterpreted his suggestion as "change the wording to this", though that wasn't his intent. http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3249 PS6, Line 3249: with an error > nit: rephrase as "non-deleted tablet which is reporting an error" or somesu Done http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3268 PS6, Line 3268: if (!cstate.committed_config().has_opid_index()) { > nit: would it be possible to move this if statement up a few lines before c Yeah, I think that'd be safe. http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3285 PS6, Line 3285: // TODO(adar): ShouldTransitionTabletToRunning doesn't take step 7b into : // account. Should it? > not sure I understand this question -- isn't the condition from ShouldTrans This TODO is trying to draw attention to the fact that ShouldTransitionTabletToRunning when examining the replica's ConsensusStatePB, goes straight to report rather than looking at the local 'cstate' (which may have had its leader_uuid() cleared). I am comfortable changing it to use 'cstate', but I wasn't sure whether that'd be correct. http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3344 PS6, Line 3344: ConsensusStatePB prev_cstate = tablet->metadata().state().pb.consensus_state(); > this is a little confusing - this is shadowing a variable with the same nam Ugh, yes. Given the COW nature of the persistent data, it should be safe to reuse the existing prev_cstate even though we're mutating the cstate. -- To view, visit http://gerrit.cloudera.org:8080/8090 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6f5cf0e4b1cd1160b3b310d89c6dbf3dd62e43b Gerrit-Change-Number: 8090 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd
[kudu-CR] KUDU-2055 [part 5]: Incorporate BlockDeletionTransaction in block deletions
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8163 ) Change subject: KUDU-2055 [part 5]: Incorporate BlockDeletionTransaction in block deletions .. Patch Set 1: (1 comment) Anything worth testing here? http://gerrit.cloudera.org:8080/#/c/8163/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8163/1//COMMIT_MSG@10 PS1, Line 10: to improve overall performance of : batched block deletions, such as delete a tablet. to improve overall performance of tablet deletion by batching up the individual blocks being deleted. -- To view, visit http://gerrit.cloudera.org:8080/8163 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1bfac61a70b45d9f27d807f22bc21e582da2dd97 Gerrit-Change-Number: 8163 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 02 Oct 2017 22:33:03 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2055 [part 4]: Coalesce hole punch for LBM
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8162 ) Change subject: KUDU-2055 [part 4]: Coalesce hole punch for LBM .. Patch Set 3: (18 comments) http://gerrit.cloudera.org:8080/#/c/8162/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8162/3//COMMIT_MSG@13 PS3, Line 13: It also adds a new metric 'holes_punched' in log block manager to track Let's also add a new counter for 'blocks_deleted', which will track the total number of deletions since startup. By comparing it with holes_punched, we can see how effective the coalescing behavior is. Actually, let's add 'blocks_deleted' and 'blocks_created' as generic block manager metrics (see block_manager_metrics.{cc,h}), since they're not specific to the LBM. http://gerrit.cloudera.org:8080/#/c/8162/3//COMMIT_MSG@14 PS3, Line 14: operation operations http://gerrit.cloudera.org:8080/#/c/8162/3//COMMIT_MSG@17 PS3, Line 17: vaule value http://gerrit.cloudera.org:8080/#/c/8162/3//COMMIT_MSG@17 PS3, Line 17: coalescing hole punch works as expected by checking the vaule of punching http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/block_manager.h@323 PS3, Line 323: class BlockDeletionTransaction { In part 3 I left a comment about how it'd be nice for these transaction classes to be purely virtual. If you agree, I think this bit of refactoring should be moved to part 3. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager-test.cc File src/kudu/fs/log_block_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager-test.cc@347 PS3, Line 347: std::shared_ptr deletion_transaction = Add a using:: for this. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@128 PS3, Line 128: METRIC_DEFINE_gauge_uint64(server, log_block_manager_holes_punched, Because the number of holes punched only ever increases, this should be a counter not a gauge. See "Gauge vs. Counter" in metrics.h for more information. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@129 PS3, Line 129:"Blocks Under Management", Update. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@130 PS3, Line 130:kudu::MetricUnit::kBlocks, Update. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@131 PS3, Line 131:"Number of data blocks currently under management"); Update. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@182 PS3, Line 182: scoped_refptr> holes_punched; Please separate from the previous two metrics with a blank line, since it's not an "under management" metric. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@229 PS3, Line 229: to with http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@230 PS3, Line 230: gets is http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@246 PS3, Line 246: that this block has been registered to with which this block has been registered. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@1208 PS3, Line 1208: if (container->block_manager()->metrics()) { : container->block_manager()->metrics()->holes_punched->IncrementBy( : entry.second.size()); : } For this to be more accurate: 1. Defer it to ContainerDeletionAsync, so that the change in metric values reflect the time that it actually takes to process the hole punches. 2. Only do it if hole punching actually succeeds. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@1224 PS3, Line 1224: Status s = lbm_->RemoveLogBlock(block, shared_from_this()); Would be better if: 1. shared_from_this() were called once; no need to call it for every iteration in the loop. 2. RemoveLogBlock() was actually RemoveLogBlocks() to avoid a spinlock acquisition/release with each loop iteration. http://gerrit.cloudera.org:8080/#/c/8162/3/src/kudu/fs/log_block_manager.cc@1286 PS3, Line 1286: transaction_->AddBlock(this); The call sequence for deleting blocks is long and involves a lot of class traversal: 1. LogBlockManager::DeletionTransaction() to get a new transaction. Also calls BlockDeletionTransaction and LogBlockDeletionTransaction constructors. 2. BlockDeletionTransaction::AddDeletedBlock to mark a block as to-be-deleted. 3. LogBlockDeletionTransaction::CommitDeletedBlocks to effect the deletions in-memory and to set them up to be deleted on disk. This calls LogBlockManager::RemoveLogBlock, w
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 ) Change subject: [tablet copy] end session before bootstrapping tablet .. Patch Set 1: > > Should this patch be abandoned? > > I think I need to add the corresponding comment and then abandon > the patch. I'll do that today. I meant I'll post a new patch to add comments on the necessity to keep the target session alive until the tablet successfully starts up. -- To view, visit http://gerrit.cloudera.org:8080/8133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20592007a40113d8409f121b226ba14e8300 Gerrit-Change-Number: 8133 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 02 Oct 2017 22:26:51 + Gerrit-HasComments: No
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 ) Change subject: [tablet copy] end session before bootstrapping tablet .. Patch Set 1: > Should this patch be abandoned? I think I need to add the corresponding comment and then abandon the patch. I'll do that today. -- To view, visit http://gerrit.cloudera.org:8080/8133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20592007a40113d8409f121b226ba14e8300 Gerrit-Change-Number: 8133 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 02 Oct 2017 22:24:08 + Gerrit-HasComments: No
[kudu-CR] [tests] fix flakes in delete table-itest
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/7972 ) Change subject: [tests] fix flakes in delete_table-itest .. Patch Set 3: Maybe we should just merge this patch the way it is to reduce the flakiness issues... thoughts? -- To view, visit http://gerrit.cloudera.org:8080/7972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7dda40ae1054becaf25963a6d301ecaed5a926f9 Gerrit-Change-Number: 7972 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 02 Oct 2017 21:53:32 + Gerrit-HasComments: No
[kudu-CR] KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8037 ) Change subject: KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries. .. Patch Set 1: Are we planning on merging this patch? -- To view, visit http://gerrit.cloudera.org:8080/8037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f47dc006dc3dfb1659a224172e1905b6bf3d2a4 Gerrit-Change-Number: 8037 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 21:52:39 + Gerrit-HasComments: No
[kudu-CR] [tablet copy] end session before bootstrapping tablet
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8133 ) Change subject: [tablet copy] end session before bootstrapping tablet .. Patch Set 1: Should this patch be abandoned? -- To view, visit http://gerrit.cloudera.org:8080/8133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I20592007a40113d8409f121b226ba14e8300 Gerrit-Change-Number: 8133 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Mon, 02 Oct 2017 21:52:19 + Gerrit-HasComments: No
[kudu-CR] WIP [raft consensus-itest] fix flake in TestSlowLeader
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/7887 ) Change subject: WIP [raft_consensus-itest] fix flake in TestSlowLeader .. Patch Set 2: I think this change makes sense but I am wondering what the purpose of the read thread is in the test, anyway. The original test doesn't do a good job of justifying its existence or implementation in my opinion. I am honestly surprised it is not more flaky than reported here. -- To view, visit http://gerrit.cloudera.org:8080/7887 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie5ee6c5400c947f87b1da2e76d24dd837b1270ca Gerrit-Change-Number: 7887 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 02 Oct 2017 21:51:57 + Gerrit-HasComments: No
[kudu-CR] KUDU-2055 [part 3]: Refactor BlockCreationTransaction and BlockDeletionTransaction
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8144 ) Change subject: KUDU-2055 [part 3]: Refactor BlockCreationTransaction and BlockDeletionTransaction .. Patch Set 5: (13 comments) http://gerrit.cloudera.org:8080/#/c/8144/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8144/4//COMMIT_MSG@10 PS4, Line 10: so that in : future they can be extended for specific implementations. Is one of the motivations also to force block creation and deletion to take place via these transactions rather than today's CreateBlock() and DeleteBlock()? If so, it would be good to understand how we'll get there, since that requires some additional refactoring. http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/cfile/bloomfile.cc File src/kudu/cfile/bloomfile.cc: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/cfile/bloomfile.cc@116 PS4, Line 116: fs::BlockManager We having "using fs::..." stuff above; you can do the same for BlockManager. http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/cfile/cfile-test.cc File src/kudu/cfile/cfile-test.cc: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/cfile/cfile-test.cc@937 PS4, Line 937: fs::BlockManager* bm = fs_manager_->block_manager(); We having "using fs::..." stuff above; you can do the same for BlockManager. http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/cfile/cfile_writer.cc File src/kudu/cfile/cfile_writer.cc: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/cfile/cfile_writer.cc@206 PS4, Line 206: fs::BlockManager* bm = block_->block_manager(); We having "using fs::..." stuff above; you can do the same for BlockManager. http://gerrit.cloudera.org:8080/#/c/8144/5/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: http://gerrit.cloudera.org:8080/#/c/8144/5/src/kudu/fs/block_manager.h@258 PS5, Line 258: CreationTransaction Nit: how about NewCreationTransaction()? And NewDeletionTransaction? http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/fs/block_manager.h@283 PS4, Line 283: class BlockCreationTransaction { I think this and BlockDeletionTransaction should be pure interfaces. It may result in some duplicated code (not much, I think), but I believe it'll let you eliminate BlockManager::CloseBlocks, which AFAICT only exists because BlockCreationTransaction isn't a member of LogBlockManager. The other problem with the transaction classes being concrete classes is that there's nothing stopping me from declaring and using them directly, when the intent of this patch is (I believe) to force people to use CreationTransaction() and DeletionTransaction(). Is this going to be addressed in a follow-on patch? http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/fs/block_manager.h@325 PS4, Line 325: explicit BlockDeletionTransaction(BlockManager* bm) > I read it at: http://en.cppreference.com/w/cpp/language/rule_of_three that The rule of three explanation makes sense, but it might be nice to define a virtual dtor just so this is more consistent with BlockCreationTransaction. It's easy to look at the two classes, see that one doesn't define a virtual dtor, and assume it's a mistake. http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/fs/file_block_manager.cc File src/kudu/fs/file_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/fs/file_block_manager.cc@801 PS4, Line 801: unique_ptr transaction( : new internal::FileBlockCreationTransaction(this)); : return std::move(transaction); Nit: combine http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/fs/log_block_manager.cc@1832 PS4, Line 1832: unique_ptr transaction( : new internal::LogBlockCreationTransaction(this)); : return std::move(transaction); Nit: combine http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/tablet/deltafile.cc File src/kudu/tablet/deltafile.cc: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/tablet/deltafile.cc@116 PS4, Line 116: fs::BlockManager* bm = writer_->block()->block_manager(); We having "using fs::..." stuff above; you can do the same for BlockManager. http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/tablet/diskrowset.cc@231 PS4, Line 231: fs::BlockManager* bm = rowset_metadata_->fs_manager()->block_manager(); We having "using fs::..." stuff above; you can do the same for BlockManager. http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/tablet/multi_column_writer.cc File src/kudu/tablet/multi_column_writer.cc: http://gerrit.cloudera.org:8080/#/c/8144/4/src/kudu/tablet/mult
[kudu-CR] [catalog manager] introduce replica selector
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8161 ) Change subject: [catalog manager] introduce replica selector .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/8161/3/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/8161/3/src/kudu/master/master.proto@378 PS3, Line 378: ReplicaSelector Bike-shedding, but "ReplicaSelector" (especially when seen in C++) suggests a stateful class that uses some sort of heuristic or algorithm to make a selection. The reality is that it's just an enum, so perhaps ReplicaSelectionMode would be a clearer name? -- To view, visit http://gerrit.cloudera.org:8080/8161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93 Gerrit-Change-Number: 8161 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 21:36:53 + Gerrit-HasComments: Yes
[kudu-CR] [webui] Allow custom response codes and headers
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8141 ) Change subject: [webui] Allow custom response codes and headers .. Patch Set 6: (9 comments) I'm not an HTTP expert so it'd be good to find a reviewer who is (maybe Todd?). I know Impala also uses Squeasel; perhaps they even use this Webserver code. Could you check in with an Impala developer and see whether their Webserver already does this, or whether they'd be interested in this functionality? http://gerrit.cloudera.org:8080/#/c/8141/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8141/6//COMMIT_MSG@14 PS6, Line 14: has have ("WebResponse and PrerenderedWebResponse" are plural) http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/master/master-path-handlers.cc File src/kudu/master/master-path-handlers.cc: http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/master/master-path-handlers.cc@242 PS6, Line 242: response respond http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h File src/kudu/server/webserver.h: http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.h@66 PS6, Line 66: // Register a route 'path' to be rendered via template. Since you're already updating these docs, could you explain what 'alias', 'is_styled' and 'is_on_nav_bar' do? http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc File src/kudu/server/webserver.cc: http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@88 PS6, Line 88: switch (code) { Add a default case that crashes or something? Would be good to know if we've forgotten to update this switch when adding a new code. http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@452 PS6, Line 452: HttpResponseHeaders{} Just {} doesn't work? http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@469 PS6, Line 469: for (const auto& entry : resp.response_headers) { Should we enforce that response_headers doesn't include Content-Type, Content-Length, or X-Frame-Options? I understand it's sort of moot since no callback responds with headers right now, but what happens if a duplicate header is in the resposne? http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/server/webserver.cc@486 PS6, Line 486: HttpResponseHeaders{} Just {} doesn't work? http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h File src/kudu/util/web_callback_registry.h: http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h@66 PS6, Line 66: typedef std::map HttpResponseHeaders; Is it important that the headers be sorted by key? Or can this be an unordered_map instead? http://gerrit.cloudera.org:8080/#/c/8141/6/src/kudu/util/web_callback_registry.h@69 PS6, Line 69: // - 'status_code' determines the status code of the HTTP response. : // - 'response_headers' are additional headers added to the HTTP response. : // - 'output' is a JSON object to be rendered in a mustache template. Nit: move each of these so that it's just above the declaration of its field. Same for 'output' in PrerenderedWebResponse. -- To view, visit http://gerrit.cloudera.org:8080/8141 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924 Gerrit-Change-Number: 8141 Gerrit-PatchSet: 6 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 02 Oct 2017 21:33:14 + Gerrit-HasComments: Yes
[kudu-CR] java: replace bespoke minicluster implementation with control shell
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8194 ) Change subject: java: replace bespoke minicluster implementation with control shell .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/8194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ia6340dadd6ff0397fffa23388a8cbbca3e26a618 Gerrit-Change-Number: 8194 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] java: replace bespoke minicluster implementation with control shell
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8194 ) Change subject: java: replace bespoke minicluster implementation with control shell .. Patch Set 1: Verified+1 Overriding Jenkins, the one test failure was due to an unsynchronized clock. -- To view, visit http://gerrit.cloudera.org:8080/8194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia6340dadd6ff0397fffa23388a8cbbca3e26a618 Gerrit-Change-Number: 8194 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 02 Oct 2017 20:55:11 + Gerrit-HasComments: No
[kudu-CR] tool: add cluster shell action
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/7853 ) Change subject: tool: add cluster shell action .. Patch Set 8: Verified+1 Overriding Jenkins, the 10 test failures were all due to an unsynchronized clock on one dist-test slave. -- To view, visit http://gerrit.cloudera.org:8080/7853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e693921ef780dc4a06e536c6b7408f7f0b252f6 Gerrit-Change-Number: 7853 Gerrit-PatchSet: 8 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 20:50:57 + Gerrit-HasComments: No
[kudu-CR] tool: add cluster shell action
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/7853 ) Change subject: tool: add cluster shell action .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/7853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I0e693921ef780dc4a06e536c6b7408f7f0b252f6 Gerrit-Change-Number: 7853 Gerrit-PatchSet: 8 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2044 Tombstoned tablets show up in /metrics
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/7981 ) Change subject: KUDU-2044 Tombstoned tablets show up in /metrics .. Patch Set 6: (6 comments) LGTM, just a couple nits http://gerrit.cloudera.org:8080/#/c/7981/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/7981/6//COMMIT_MSG@20 PS6, Line 20: tombstoned's tablets tombstoned tablet's http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/integration-tests/tablet_copy-itest.cc File src/kudu/integration-tests/tablet_copy-itest.cc: http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/integration-tests/tablet_copy-itest.cc@1726 PS6, Line 1726: // Find the leader so we can tombstone a follower, which makes the test faster and more focused. : int leader_index; : int follower_index; : TServerDetails* leader_ts; : TServerDetails* follower_ts; : ASSERT_OK(FindTabletLeader(ts_map_, tablet_id, kTimeout, &leader_ts)); : leader_index = cluster_->tablet_server_index_by_uuid(leader_ts->uuid()); : follower_index = (leader_index + 1) % cluster_->num_tablet_servers(); : follower_ts = ts_map_[cluster_->tablet_server(follower_index)->uuid()]; : : // Make sure the metrics reflect that some rows have been inserted. : ASSERT_GT(CountRowsInserted(cluster_->tablet_server(follower_index)), 0); : : // Tombstone the tablet on the follower. : ASSERT_OK(itest::DeleteTablet(follower_ts, tablet_id, : TABLET_DATA_TOMBSTONED, : boost::none, kTimeout)); This bit should be wrapped in ASSERT_EVENTUALLY() { ... } in order to ensure that the test doesn't get flaky due to typical latency volatility on the test machines. http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/tserver/tablet_server-test.cc@2583 PS6, Line 2583: It takes three calls Thanks for the explanation. Can you please add: "at the time of writing, based on the policy in ClassFoo" or "foo.cc", or something like that? http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/tserver/tablet_server-test.cc@2591 PS6, Line 2591: mini nit: indent this another 4 spaces to line up with the parameter on the above line http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/tserver/tablet_server-test.cc@2592 PS6, Line 2592: &buf nit: the indentation of this line seems random http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/util/metrics.h File src/kudu/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/7981/6/src/kudu/util/metrics.h@544 PS6, Line 544: bool unpublished_; Sometimes thinking in negatives can be confusing. How about flipping this and the corresponding getter method to "published" ? -- To view, visit http://gerrit.cloudera.org:8080/7981 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I21b52262203410ded1e514502b59a79be12f8fb3 Gerrit-Change-Number: 7981 Gerrit-PatchSet: 6 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 02 Oct 2017 19:56:33 + Gerrit-HasComments: Yes
[kudu-CR] java: replace bespoke minicluster implementation with control shell
Hello Dan Burkert, Jean-Daniel Cryans, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8194 to review the following change. Change subject: java: replace bespoke minicluster implementation with control shell .. java: replace bespoke minicluster implementation with control shell This patch replaces the Java client's bespoke MiniKuduCluster and MiniKdc implementations with appropriate calls to the new CLI control shell. The shell can communicate either in protobuf or JSON; I've opted to use the former here since the Java client already understands protobuf, and doing so yields more type safety. Change-Id: Ia6340dadd6ff0397fffa23388a8cbbca3e26a618 --- M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java D java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java D java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKdc.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java D java/kudu-client/src/test/resources/flags M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala 10 files changed, 409 insertions(+), 1,233 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/8194/1 -- To view, visit http://gerrit.cloudera.org:8080/8194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia6340dadd6ff0397fffa23388a8cbbca3e26a618 Gerrit-Change-Number: 8194 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] tool: add cluster shell action
Hello Tidy Bot, Alexey Serbin, Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7853 to look at the new patch set (#8). Change subject: tool: add cluster shell action .. tool: add cluster shell action Maintaining Kudu clients across various languages has been an ongoing maintenance burden. Even when the client is just a thin wrapper around another client (e.g. Kudu Python bindings), a great deal of work goes into client testability. In practice, this has meant a bespoke mini cluster implementation for each language. On the surface this doesn't seem that bad; we just need to spawn some masters and tservers, right? Well, the work quickly adds up: o While the C++ mini cluster is heavily used and has seen many improvements, the Java mini cluster has not received the same kind of love, and is less robust as a result. KUDU-1976 is a great example of this deficiency. o With the inclusion of authn came the addition of a "mini KDC", a special daemon for Kerberized mini clusters. It was originally implemented in C++ and ported to Java, but has yet to be ported to the Python client; this is one of the obstacles towards porting full authn support to Python. o Dan has been prototyping Hive Metastore and Sentry integration for Kudu, the testing of which will require "mini HMS" and possibly "mini Sentry" testing implementations in C++, Java, and eventually, Python. In sum, good support for non-C++ mini clusters is an ongoing commitment and requires a great deal of work. This work hasn't always been forthcoming, and the non-C++ clusters are deficient as a result. But it doesn't have to be this way! Here's a thought: what if we reused the C++ mini cluster for tests written in these other languages? We could write a "proxy" application whose job it is to manage the C++ mini cluster and expose a rudimentary API that's easily programmable from Java and Python. This patch attempts to do just that. It adds a "shell" action to the Kudu CLI which provides a rudimentary control shell that can be used to spin up an ExternalMiniCluster. The shell is controlled via a wire protocol over stdin/stdout. The protocol is protobuf-based with optional JSON encoding. I should add that I like the idea of shipping "shell" into production as part of the CLI, as it helps realize the vision of a single Kudu artifact that can provide Kudu testability for any integrating product. Change-Id: I0e693921ef780dc4a06e536c6b7408f7f0b252f6 --- M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-tool-test.cc A src/kudu/tools/tool.proto M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h A src/kudu/tools/tool_action_test.cc M src/kudu/tools/tool_main.cc 9 files changed, 1,129 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7853/8 -- To view, visit http://gerrit.cloudera.org:8080/7853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e693921ef780dc4a06e536c6b7408f7f0b252f6 Gerrit-Change-Number: 7853 Gerrit-PatchSet: 8 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] subprocess: some cosmetic changes
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8157 ) Change subject: subprocess: some cosmetic changes .. subprocess: some cosmetic changes Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d Reviewed-on: http://gerrit.cloudera.org:8080/8157 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M src/kudu/util/subprocess.cc M src/kudu/util/subprocess.h 2 files changed, 33 insertions(+), 19 deletions(-) Approvals: Kudu Jenkins: Verified Dan Burkert: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d Gerrit-Change-Number: 8157 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] mini-cluster: new module for the mini cluster implementations
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8173 ) Change subject: mini-cluster: new module for the mini cluster implementations .. mini-cluster: new module for the mini cluster implementations For the upcoming "control shell" patch, the Kudu CLI will need to link ExternalMiniCluster and friends. Today that means depending on the integration-tests target, which doesn't feel quite right since that's mostly, well, integration tests. More practically, the integration-tests cmake target is conditioned on !NO_TESTS but the Kudu CLI is not. So this patch moves the mini cluster code into a standalone module. I also took the opportunity to place the code in a new "cluster" namespace. Change-Id: I0437e281da5874016d9c1f1404a6de043bfb4088 Reviewed-on: http://gerrit.cloudera.org:8080/8173 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M CMakeLists.txt M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/benchmarks/tpch/tpch_real_world.cc M src/kudu/client/CMakeLists.txt M src/kudu/client/client-test.cc M src/kudu/client/predicate-test.cc M src/kudu/client/scan_token-test.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/all_types-itest.cc M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/authn_token_expire-itest.cc M src/kudu/integration-tests/catalog_manager_tsk-itest.cc M src/kudu/integration-tests/client-negotiation-failover-itest.cc M src/kudu/integration-tests/client-stress-test.cc M src/kudu/integration-tests/client_failover-itest.cc M src/kudu/integration-tests/cluster_verifier.cc M src/kudu/integration-tests/cluster_verifier.h M src/kudu/integration-tests/consistency-itest.cc M src/kudu/integration-tests/create-table-itest.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/delete_table-itest.cc M src/kudu/integration-tests/delete_tablet-itest.cc M src/kudu/integration-tests/dense_node-itest.cc M src/kudu/integration-tests/disk_failure-itest.cc M src/kudu/integration-tests/disk_reservation-itest.cc M src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.cc M src/kudu/integration-tests/external_mini_cluster-itest-base.h M src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc M src/kudu/integration-tests/external_mini_cluster_fs_inspector.h M src/kudu/integration-tests/flex_partitioning-itest.cc M src/kudu/integration-tests/full_stack-insert-scan-test.cc M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/integration-tests/internal_mini_cluster-itest-base.cc M src/kudu/integration-tests/internal_mini_cluster-itest-base.h M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/log-rolling-itest.cc M src/kudu/integration-tests/log_verifier.cc M src/kudu/integration-tests/log_verifier.h M src/kudu/integration-tests/master-stress-test.cc M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/minidump_generation-itest.cc M src/kudu/integration-tests/multidir_cluster-itest.cc M src/kudu/integration-tests/open-readonly-fs-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/registration-test.cc M src/kudu/integration-tests/security-faults-itest.cc M src/kudu/integration-tests/security-itest.cc M src/kudu/integration-tests/security-unknown-tsk-itest.cc M src/kudu/integration-tests/table_locations-itest.cc M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/integration-tests/tablet_copy_client_session-itest.cc M src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/test_workload.cc M src/kudu/integration-tests/test_workload.h M src/kudu/integration-tests/token_signer-itest.cc M src/kudu/integration-tests/tombstoned_voting-imc-itest.cc M src/kudu/integration-tests/tombstoned_voting-itest.cc M src/kudu/integration-tests/tombstoned_voting-stress-test.cc M src/kudu/integration-tests/ts_itest-base.h M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/integration-tests/version_migration-test.cc M src/kudu/integration-tests/webserver-stress-itest.cc A src/kudu/mini-cluster/CMakeLists.txt R src/kudu/mini-cluster/external_mini_cluster-test.cc R src/kudu/mini-cluster/external_mini_cluster.cc R src/kudu/mini-cluster/external_mini_cluster.h R src/kudu/mini-
[kudu-CR] KUDU-2132 Error dropping columns renamed to old key column names
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8188 ) Change subject: KUDU-2132 Error dropping columns renamed to old key column names .. KUDU-2132 Error dropping columns renamed to old key column names Suppose a table has two columns 'a' and 'b', and 'a' is the primary key. Consider the following sequence of alter table steps, done as part of one alter table request: 1. Rename the key column 'a' to 'c'. 2. Rename 'b' to 'a', the previous name of the key column. 3. Drop 'a'. This is a valid sequence, but previously we were rejecting it, saying that a primary key column was being dropped, even though that isn't the case. The problem is that CatalogManager::ApplyAlterSchemaSteps checked whether a column was a key column by checking by name against the table's schema before any alter steps were applied. The fix is to check against the schema with the previous alter steps applied. Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2 Reviewed-on: http://gerrit.cloudera.org:8080/8188 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo Reviewed-by: Dan Burkert --- M src/kudu/common/schema.h M src/kudu/integration-tests/alter_table-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 4 files changed, 21 insertions(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, but someone else must approve Dan Burkert: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8188 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2 Gerrit-Change-Number: 8188 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2132 Error dropping columns renamed to old key column names
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8188 ) Change subject: KUDU-2132 Error dropping columns renamed to old key column names .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8188 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2 Gerrit-Change-Number: 8188 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 02 Oct 2017 18:53:58 + Gerrit-HasComments: No
[kudu-CR] tool: add cluster shell action
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/7853 ) Change subject: tool: add cluster shell action .. Patch Set 7: (8 comments) http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/kudu-tool-test.cc@89 PS7, Line 89: #include "kudu/tools/tool_action_common.h" > warning: #includes are not sorted properly [llvm-include-order] Done http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_common.h File src/kudu/tools/tool_action_common.h: http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_common.h@39 PS7, Line 39: class faststring; > warning: invalid case style for class 'faststring' [readability-identifier- It's a forward declaration of an existing class... http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_common.cc File src/kudu/tools/tool_action_common.cc: http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_common.cc@38 PS7, Line 38: #include "kudu/client/client.h" > warning: #includes are not sorted properly [llvm-include-order] Done http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_common.cc@148 PS7, Line 148: using tserver::TabletServerAdminServiceProxy; > warning: using decl 'TabletServerAdminServiceProxy' is unused [misc-unused- This is used in template specializations in this file. If I remove it, the build fails. http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_common.cc@149 PS7, Line 149: using tserver::TabletServerServiceProxy; > warning: using decl 'TabletServerServiceProxy' is unused [misc-unused-using This is used in template specializations in this file. If I remove it, the build fails. http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_test.cc File src/kudu/tools/tool_action_test.cc: http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_test.cc@76 PS7, Line 76: using std::cin; > warning: using decl 'cin' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_test.cc@77 PS7, Line 77: using std::cout; > warning: using decl 'cout' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/7853/7/src/kudu/tools/tool_action_test.cc@78 PS7, Line 78: using std::endl; > warning: using decl 'endl' is unused [misc-unused-using-decls] Done -- To view, visit http://gerrit.cloudera.org:8080/7853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e693921ef780dc4a06e536c6b7408f7f0b252f6 Gerrit-Change-Number: 7853 Gerrit-PatchSet: 7 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 18:49:46 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2132 Error dropping columns renamed to old key column names
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8188 ) Change subject: KUDU-2132 Error dropping columns renamed to old key column names .. Patch Set 2: Code-Review+1 Will defer to Dan. -- To view, visit http://gerrit.cloudera.org:8080/8188 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2 Gerrit-Change-Number: 8188 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 02 Oct 2017 18:47:34 + Gerrit-HasComments: No
[kudu-CR] WIP [consensus] adding/removing NON VOTER members
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8138 ) Change subject: WIP [consensus] adding/removing NON_VOTER members .. Patch Set 2: (2 comments) Thank you for the review! http://gerrit.cloudera.org:8080/#/c/8138/2/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: http://gerrit.cloudera.org:8080/#/c/8138/2/src/kudu/integration-tests/raft_consensus-itest.cc@416 PS2, Line 416: // Get number of source tablet copy sessions at the specified server. > nit: leave one blank line above this, for consistency. Done http://gerrit.cloudera.org:8080/#/c/8138/2/src/kudu/integration-tests/raft_consensus-itest.cc@3242 PS2, Line 3242: TEST_F(RaftConsensusITest, AddNonVoterReplica) { > I think this is a good addition, but that, ultimately we also need to add n Yep, that's a good idea -- I'm working on it. -- To view, visit http://gerrit.cloudera.org:8080/8138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2662d45ad9bb6a4bf325d4202c2ee619ffad02b7 Gerrit-Change-Number: 8138 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 18:35:13 + Gerrit-HasComments: Yes
[kudu-CR] WIP [consensus] adding/removing NON VOTER members
Hello Tidy Bot, Mike Percy, David Ribeiro Alves, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8138 to look at the new patch set (#5). Change subject: WIP [consensus] adding/removing NON_VOTER members .. WIP [consensus] adding/removing NON_VOTER members Added ability to add and remove NON_VOTER member replicas. Updated the kudu CLI accordingly. Also, added new integration test: * RaftConsensusITest.AddNonVoterReplica * RaftConsensusITest.AddThenRemoveNonVoterReplica WIP: Perhaps, I should continue gathering feedback on this patch and adding more tests. Change-Id: I2662d45ad9bb6a4bf325d4202c2ee619ffad02b7 --- M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/leader_election.cc M src/kudu/consensus/leader_election.h M src/kudu/consensus/metadata.proto M src/kudu/consensus/quorum_util-test.cc M src/kudu/consensus/quorum_util.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/raft_consensus-itest.cc 9 files changed, 431 insertions(+), 58 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/8138/5 -- To view, visit http://gerrit.cloudera.org:8080/8138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2662d45ad9bb6a4bf325d4202c2ee619ffad02b7 Gerrit-Change-Number: 8138 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] WIP [consensus] adding/removing NON VOTER members
Hello Tidy Bot, Mike Percy, David Ribeiro Alves, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8138 to look at the new patch set (#4). Change subject: WIP [consensus] adding/removing NON_VOTER members .. WIP [consensus] adding/removing NON_VOTER members Added ability to add and remove NON_VOTER member replicas. Updated the kudu CLI accordingly. Also, added new integration test: * RaftConsensusITest.AddNonVoterReplica * RaftConsensusITest.AddThenRemoveNonVoterReplica WIP: Perhaps, I should continue gathering feedback on this patch and adding more tests. Change-Id: I2662d45ad9bb6a4bf325d4202c2ee619ffad02b7 --- M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/leader_election.cc M src/kudu/consensus/leader_election.h M src/kudu/consensus/metadata.proto M src/kudu/consensus/quorum_util-test.cc M src/kudu/consensus/quorum_util.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/raft_consensus-itest.cc 9 files changed, 430 insertions(+), 58 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/8138/4 -- To view, visit http://gerrit.cloudera.org:8080/8138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2662d45ad9bb6a4bf325d4202c2ee619ffad02b7 Gerrit-Change-Number: 8138 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.3.x) KUDU-2032 (part 2): propagate master hostnames into client
Hello Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8183 to look at the new patch set (#3). Change subject: KUDU-2032 (part 2): propagate master hostnames into client .. KUDU-2032 (part 2): propagate master hostnames into client This changes the client code to remember the user-specified master addresses and propagate them into the creation of master proxies. It's not possible to reproduce the necessary DNS configurations in a minicluster test, but with this patch I am now able to use 'kudu perf loadgen' against a Kerberized cluster even when my local krb5.conf has rdns=false. Change-Id: I0c3e6c5f6543a86173e242b04d3515f5ec69200d --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/integration-tests/external_mini_cluster.cc 5 files changed, 68 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/8183/3 -- To view, visit http://gerrit.cloudera.org:8080/8183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.3.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0c3e6c5f6543a86173e242b04d3515f5ec69200d Gerrit-Change-Number: 8183 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.4.x) KUDU-2032 (part 2): propagate master hostnames into client
Hello Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8186 to look at the new patch set (#3). Change subject: KUDU-2032 (part 2): propagate master hostnames into client .. KUDU-2032 (part 2): propagate master hostnames into client This changes the client code to remember the user-specified master addresses and propagate them into the creation of master proxies. It's not possible to reproduce the necessary DNS configurations in a minicluster test, but with this patch I am now able to use 'kudu perf loadgen' against a Kerberized cluster even when my local krb5.conf has rdns=false. Change-Id: I4bd299f72907a0d9442dd3eb45fcbaa2de4f73ef --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/integration-tests/external_mini_cluster.cc 5 files changed, 58 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/8186/3 -- To view, visit http://gerrit.cloudera.org:8080/8186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.4.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4bd299f72907a0d9442dd3eb45fcbaa2de4f73ef Gerrit-Change-Number: 8186 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.3.x) KUDU-2032 (part 2): propagate master hostnames into client
Hello Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8183 to look at the new patch set (#2). Change subject: KUDU-2032 (part 2): propagate master hostnames into client .. KUDU-2032 (part 2): propagate master hostnames into client This changes the client code to remember the user-specified master addresses and propagate them into the creation of master proxies. It's not possible to reproduce the necessary DNS configurations in a minicluster test, but with this patch I am now able to use 'kudu perf loadgen' against a Kerberized cluster even when my local krb5.conf has rdns=false. Change-Id: I0c3e6c5f6543a86173e242b04d3515f5ec69200d --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/integration-tests/external_mini_cluster.cc 5 files changed, 68 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/8183/2 -- To view, visit http://gerrit.cloudera.org:8080/8183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.3.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0c3e6c5f6543a86173e242b04d3515f5ec69200d Gerrit-Change-Number: 8183 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2032 (part 2): propagate master hostnames into client
Will Berkeley has abandoned this change. ( http://gerrit.cloudera.org:8080/8193 ) Change subject: KUDU-2032 (part 2): propagate master hostnames into client .. Abandoned Whoops. Wrong branch. -- To view, visit http://gerrit.cloudera.org:8080/8193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I4bd299f72907a0d9442dd3eb45fcbaa2de4f73ef Gerrit-Change-Number: 8193 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2032 (part 2): propagate master hostnames into client
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8193 Change subject: KUDU-2032 (part 2): propagate master hostnames into client .. KUDU-2032 (part 2): propagate master hostnames into client This changes the client code to remember the user-specified master addresses and propagate them into the creation of master proxies. It's not possible to reproduce the necessary DNS configurations in a minicluster test, but with this patch I am now able to use 'kudu perf loadgen' against a Kerberized cluster even when my local krb5.conf has rdns=false. Change-Id: I4bd299f72907a0d9442dd3eb45fcbaa2de4f73ef --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/integration-tests/external_mini_cluster.cc 5 files changed, 58 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/8193/1 -- To view, visit http://gerrit.cloudera.org:8080/8193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4bd299f72907a0d9442dd3eb45fcbaa2de4f73ef Gerrit-Change-Number: 8193 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-1.4.x) KUDU-2032 (part 1): pass pre-resolution hostname into RPC proxies
Hello Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8185 to look at the new patch set (#2). Change subject: KUDU-2032 (part 1): pass pre-resolution hostname into RPC proxies .. KUDU-2032 (part 1): pass pre-resolution hostname into RPC proxies This modifies the constructor of RPC proxies (generated and otherwise) to take the remote hostname in addition to the existing resolved Sockaddr parameter. The hostname is then passed into the ConnectionId object, and plumbed through to the SASL client in place of the IP address that was used previously. The patch changes all of the construction sites of Proxy to fit the new interface. In most of the test cases, we don't have real hostnames, so we just use the dotted-decimal string form of the remote Sockaddr, which matches the existing behavior. In the real call sites, we have actual host names typically specified by the user, and in those cases we'll need to pass those into the proxy. In a few cases, they were conveniently available in the same function that creates the proxy. In others, they are relatively far away, so this patch just uses the dotted-decimal string and leaves TODOs. In the case that Kerberos is not configured, this change should have no effect since the hostname is ignored by SASL "plain". In the case that Kerberos is configured with 'rdns=true', they also have no effect, because the krb5 library will resolve and reverse the hostname from the IP as it did before. In the case that 'rdns=false', this moves us one step closer to fixing KUDU-2032 by getting a hostname into the SASL library. I verified that, if I set 'rdns = false' on a Kerberized client, I'm now able to run 'kudu master status ' successfully where it would not before. This tool uses a direct proxy instantiation where the hostname was easy to plumb in. 'kudu table list ' still does not work because it uses the client, which wasn't convenient to plumb quite yet. Given that this makes incremental improvement towards fixing the issue without any regression, and is already a fairly wide patch, I hope to commit this and then address the remaining plumbing in a separate patch. Change-Id: I6d58abbe6a5d9fc524e54e4182402c5326a60d82 --- M src/kudu/client/client-internal.cc M src/kudu/client/master_rpc.cc M src/kudu/client/meta_cache.cc M src/kudu/consensus/consensus_peers.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/master-stress-test.cc M src/kudu/integration-tests/master_cert_authority-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/integration-tests/mini_cluster.cc M src/kudu/integration-tests/security-itest.cc M src/kudu/integration-tests/table_locations-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/master-test.cc M src/kudu/master/master.cc M src/kudu/master/sys_catalog-test.cc M src/kudu/master/ts_descriptor.cc M src/kudu/master/ts_descriptor.h M src/kudu/rpc/connection.h M src/kudu/rpc/connection_id.cc M src/kudu/rpc/connection_id.h M src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/mt-rpc-test.cc M src/kudu/rpc/negotiation.cc M src/kudu/rpc/protoc-gen-krpc.cc M src/kudu/rpc/proxy.cc M src/kudu/rpc/proxy.h M src/kudu/rpc/reactor.cc M src/kudu/rpc/rpc-bench.cc M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/rpc_stub-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/tablet_copy_client.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_server_test_util.cc M src/kudu/util/net/net_util-test.cc M src/kudu/util/net/sockaddr.cc M src/kudu/util/net/sockaddr.h 42 files changed, 217 insertions(+), 164 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/85/8185/2 -- To view, visit http://gerrit.cloudera.org:8080/8185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.4.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d58abbe6a5d9fc524e54e4182402c5326a60d82 Gerrit-Change-Number: 8185 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2132 Error dropping columns renamed to old key column names
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8188 to look at the new patch set (#2). Change subject: KUDU-2132 Error dropping columns renamed to old key column names .. KUDU-2132 Error dropping columns renamed to old key column names Suppose a table has two columns 'a' and 'b', and 'a' is the primary key. Consider the following sequence of alter table steps, done as part of one alter table request: 1. Rename the key column 'a' to 'c'. 2. Rename 'b' to 'a', the previous name of the key column. 3. Drop 'a'. This is a valid sequence, but previously we were rejecting it, saying that a primary key column was being dropped, even though that isn't the case. The problem is that CatalogManager::ApplyAlterSchemaSteps checked whether a column was a key column by checking by name against the table's schema before any alter steps were applied. The fix is to check against the schema with the previous alter steps applied. Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2 --- M src/kudu/common/schema.h M src/kudu/integration-tests/alter_table-test.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h 4 files changed, 21 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/8188/2 -- To view, visit http://gerrit.cloudera.org:8080/8188 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2 Gerrit-Change-Number: 8188 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2132 Error dropping columns renamed to old key column names
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8188 ) Change subject: KUDU-2132 Error dropping columns renamed to old key column names .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/8188/1/src/kudu/common/schema.h File src/kudu/common/schema.h: http://gerrit.cloudera.org:8080/#/c/8188/1/src/kudu/common/schema.h@903 PS1, Line 903: return col - cols_.begin() < num_key_columns_; > This seems a little magical to me, though maybe it's because I'm easily con Done http://gerrit.cloudera.org:8080/#/c/8188/1/src/kudu/integration-tests/alter_table-test.cc File src/kudu/integration-tests/alter_table-test.cc: http://gerrit.cloudera.org:8080/#/c/8188/1/src/kudu/integration-tests/alter_table-test.cc@2107 PS1, Line 2107: gscoped_ptr table_alterer(client_->NewTableAlterer(kTableName)); > Nit: unique_ptr for new code? Done http://gerrit.cloudera.org:8080/#/c/8188/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/8188/1/src/kudu/master/catalog_manager.cc@1761 PS1, Line 1761: if (builder.is_key_column(step.drop_column().name())) { > Is my understanding correct that L1726 initializes the builder with the cur That's right. -- To view, visit http://gerrit.cloudera.org:8080/8188 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2 Gerrit-Change-Number: 8188 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 02 Oct 2017 16:47:50 + Gerrit-HasComments: Yes
[kudu-CR] [webui] Allow custom response codes and headers
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8141 to look at the new patch set (#6). Change subject: [webui] Allow custom response codes and headers .. [webui] Allow custom response codes and headers Previously, the path handlers used to implement pages in the web ui could only return 200 OK and could not set any headers, as these two aspects of the HTTP response were handled in the underlying webserver code. This patch introduces WebResponse and PrerenderedWebResponse structs that wrap and replace the 'output' EasyJson and ostringstream pointers, respectively, used before, and which has fields for the response code and additional headers. The ability to add headers isn't currently used, but it's nice to have. The response codes are adjusted where necessary to match what one would expect, e.g. navigating to /tablet?id=foo returns 404. Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924 --- M src/kudu/master/master-path-handlers.cc M src/kudu/master/master-path-handlers.h M src/kudu/server/default-path-handlers.cc M src/kudu/server/pprof-path-handlers.cc M src/kudu/server/rpcz-path-handler.cc M src/kudu/server/tracing-path-handlers.cc M src/kudu/server/webserver.cc M src/kudu/server/webserver.h M src/kudu/tserver/tserver-path-handlers.cc M src/kudu/tserver/tserver-path-handlers.h M src/kudu/util/thread.cc M src/kudu/util/web_callback_registry.h 12 files changed, 247 insertions(+), 132 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/8141/6 -- To view, visit http://gerrit.cloudera.org:8080/8141 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9ff890785eeb2df3eed9e7c54d0daf760c8b3924 Gerrit-Change-Number: 8141 Gerrit-PatchSet: 6 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2159 Add metric for upserts converted to updates
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8177 ) Change subject: KUDU-2159 Add metric for upserts converted to updates .. KUDU-2159 Add metric for upserts converted to updates This adds a metric tracking the number of successful upsert operations that were applied as updates, per tablet, counting since server start. Change-Id: Iefdee21fbfdfd3c9c8dca4c06d635b89bcf15d72 Reviewed-on: http://gerrit.cloudera.org:8080/8177 Tested-by: Kudu Jenkins Reviewed-by: Will Berkeley --- M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet_metrics.cc M src/kudu/tablet/tablet_metrics.h 4 files changed, 19 insertions(+), 3 deletions(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iefdee21fbfdfd3c9c8dca4c06d635b89bcf15d72 Gerrit-Change-Number: 8177 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2159 Add metric for upserts converted to updates
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8177 ) Change subject: KUDU-2159 Add metric for upserts converted to updates .. Patch Set 2: Code-Review+2 Forwarding Todd's +2 since the test failures seemed to be unrelated Python/Jenkins flake. -- To view, visit http://gerrit.cloudera.org:8080/8177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iefdee21fbfdfd3c9c8dca4c06d635b89bcf15d72 Gerrit-Change-Number: 8177 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 02 Oct 2017 16:30:40 + Gerrit-HasComments: No
[kudu-CR] external mini cluster: don't pipe daemon subprocess stdout
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8187 ) Change subject: external_mini_cluster: don't pipe daemon subprocess stdout .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/8187 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Iddd8d78cd085975821207d324ea2fd5365a2c2ba Gerrit-Change-Number: 8187 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Todd Lipcon
[kudu-CR] external mini cluster: don't pipe daemon subprocess stdout
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8187 ) Change subject: external_mini_cluster: don't pipe daemon subprocess stdout .. Patch Set 2: Verified+1 Overriding Jenkins, there was an unrelated TSAN failure for which I filed KUDU-2165. -- To view, visit http://gerrit.cloudera.org:8080/8187 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iddd8d78cd085975821207d324ea2fd5365a2c2ba Gerrit-Change-Number: 8187 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 16:08:52 + Gerrit-HasComments: No
[kudu-CR] periodic: add one-shot timers
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8130 ) Change subject: periodic: add one-shot timers .. Patch Set 3: Verified+1 Overriding Jenkins, got clock synchronization errors in some of the distributed tests. -- To view, visit http://gerrit.cloudera.org:8080/8130 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4d9376172d66c92958071d5abbac63d751e41f3 Gerrit-Change-Number: 8130 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 16:01:30 + Gerrit-HasComments: No
[kudu-CR] periodic: add one-shot timers
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8130 ) Change subject: periodic: add one-shot timers .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/8130 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ia4d9376172d66c92958071d5abbac63d751e41f3 Gerrit-Change-Number: 8130 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] subprocess: some cosmetic changes
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 ) Change subject: subprocess: some cosmetic changes .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d Gerrit-Change-Number: 8157 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 15:26:34 + Gerrit-HasComments: No
[kudu-CR] mini-cluster: new module for the mini cluster implementations
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8173 ) Change subject: mini-cluster: new module for the mini cluster implementations .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0437e281da5874016d9c1f1404a6de043bfb4088 Gerrit-Change-Number: 8173 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 02 Oct 2017 15:26:10 + Gerrit-HasComments: No