[kudu-CR] Remove pessimizing std::move() in return statement

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Remove pessimizing std::move() in return statement
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3064/

-- 
To view, visit http://gerrit.cloudera.org:8080/4122
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida46588398ca5f5982cb86aa03825530565c0923
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] tool: add ksck

2016-08-24 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: tool: add ksck
..


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4121
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad9aaffa5c0c77080dcaeb2cdfa572dcbeeb1407
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] tool: add ksck

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: tool: add ksck
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3063/

-- 
To view, visit http://gerrit.cloudera.org:8080/4121
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad9aaffa5c0c77080dcaeb2cdfa572dcbeeb1407
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: Rolling-upgrades compat for changing TSRegistrationPB
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4062/5/src/kudu/master/ts_descriptor.cc
File src/kudu/master/ts_descriptor.cc:

PS5, Line 60: static bool HostPortPBsEqual(const 
::google::protobuf::RepeatedPtrField<::kudu::HostPortPB>& pb1,
:  const 
::google::protobuf::RepeatedPtrField<::kudu::HostPortPB>& pb2)
Not related to your change, but can you get away with HostPortPB instead of 
::kudu::HostPortPB? I thought namespace resolution automatically checked parent 
namespaces.


http://gerrit.cloudera.org:8080/#/c/4062/5/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:

PS5, Line 55:   bool Equals(const HostPort& other) const {
: return port_ == other.port_ && host_ == other.host_;
:   }
Since this is new, consider dropping it in favor of just operator==, which is 
pretty self-explanatory.


Line 59:   bool operator==(const HostPort& other) const {
Google style guide says const binary operators should be defined as free 
functions, so that implicit constructors (if any) can apply to both operands.


-- 
To view, visit http://gerrit.cloudera.org:8080/4062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Rolling-upgrades compat for changing TSRegistrationPB
..


Patch Set 5:

Build Started http://104.196.14.100/job/kudu-gerrit/3062/

-- 
To view, visit http://gerrit.cloudera.org:8080/4062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB

2016-08-24 Thread Mike Percy (Code Review)
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4062

to look at the new patch set (#5).

Change subject: Rolling-upgrades compat for changing TSRegistrationPB
..

Rolling-upgrades compat for changing TSRegistrationPB

This should allow for adding fields to TSRegistrationPB.
Only the host/port is checked.

Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
---
M src/kudu/master/master-test.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
4 files changed, 72 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/4062/5
-- 
To view, visit http://gerrit.cloudera.org:8080/4062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB

2016-08-24 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: Rolling-upgrades compat for changing TSRegistrationPB
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4062/4/src/kudu/master/ts_descriptor.cc
File src/kudu/master/ts_descriptor.cc:

PS4, Line 68:   using HostPortSet = std::unordered_set;
:   HostPortSet hostports;
> Interesting, but why not just:
Ah, I was planning on having to refer to the type and ended up never having to. 
Removed.


PS4, Line 70:   for (int i = 0; i < pb1.size(); i++) {
: HostPort hp1;
: if (!HostPortFromPB(pb1.Get(i), ).ok()) return false;
: hostports.insert(hp1);
:   }
:   for (int i = 0; i < pb1.size(); i++) {
: HostPort hp2;
: if (!HostPortFromPB(pb2.Get(i), ).ok()) return false;
: if (!ContainsKey(hostports, hp2)) {
:   return false;
: }
:   }
> You may be able to get away with unordered_sets of HostPortPB (maybe the ge
This would have been really nice, but it doesn't work. I think it's probably 
easy with proto3. I partially took your advice with the unordered_set equality 
thing though.


-- 
To view, visit http://gerrit.cloudera.org:8080/4062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..


Patch Set 15:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/3952/15/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 1146:   Status SetMutationBufferFlushWatermark(double watermark_pct)
> Will do.  BTW, do we need getters for those parameters?  I think getters mi
I don't think it's a big deal either way. Sessions are cheap (especially now 
that they don't have a dedicated thread to flush in the background) and so I 
tend to think they're short-lived, which means it's easy for the application to 
remember what value was provided here.


http://gerrit.cloudera.org:8080/#/c/3952/15/src/kudu/client/session-internal.cc
File src/kudu/client/session-internal.cc:

PS15, Line 64: -1,
> This is to force getting a new batcher and flushing the prior, if any, even
I don't think the existing behavior is necessarily worth preserving; an empty 
flush doesn't actually accomplish anything.

In any case, that wasn't what my comment was about; I was asking about the 
semantic difference between '-1' and '0' as the watermark value.


Line 66: PeriodicFlushTask(Status::OK(), messenger_, session);
> OK, I'll change it then.
Sorry, I don't understand what you're suggesting. Can you explain it in more 
detail?


Line 109:   flow_control_cond_.Broadcast();
> The original idea was to get rid of those limitations like empty buffer whe
Nope, simplifying the implementation is the underlying motivation. But I think 
it's enough; the clients are generally complicated code and aren't as well 
tested as the rest of Kudu.


Line 135:   CHECK_GE(timeout_ms, 0);
> Yep.  What we can do instead is to set it to 0 if the specified parameter w
I think that's reasonable provided it's documented.


PS15, Line 230:   RETURN_NOT_OK(TryApplyWriteOp(write_op, _size,
  : _flush_mode, 
_flush_watermark));
> Yep, having those facts simplifies this.  Again, I was under impression the
I don't understand why friendship factors into this: both ApplyWriteOp and 
TryApplyWriteOp are members of KuduSession::Data, so they should be equivalent 
w.r.t. calling Batcher::SizeInBuffer().


Line 398:   data->NextBatcher(session, 0, nullptr);
> Nope.  It would, however, if the second parameter were -1 (as you noticed t
I find the semantics of -1, 0, and >0 to be non-obvious. Perhaps we can convey 
the desired behavior from caller to PeriodicFlushTask() in another way? Through 
an additional parameter, an enum, or something like that?


http://gerrit.cloudera.org:8080/#/c/3952/15/src/kudu/client/session-internal.h
File src/kudu/client/session-internal.h:

PS15, Line 87: On successful return, the output flush_mode parameter is set
 :   // to the effective flush mode.
> This is because the flush_mode_ can change in the middle and the code which
I don't understand how flush_mode_ can change in the middle of ApplyWriteOp if 
the session may only be used by a single thread. Or is this a holdover from 
before, when it was OK for multiple threads to write to a session?


-- 
To view, visit http://gerrit.cloudera.org:8080/3952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1500: Data race in 
RaftConsensusITest.TestCorruptReplicaMetadata
..


Patch Set 10:

Build Started http://104.196.14.100/job/kudu-gerrit/3061/

-- 
To view, visit http://gerrit.cloudera.org:8080/3823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] tool: add ksck

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: tool: add ksck
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3060/

-- 
To view, visit http://gerrit.cloudera.org:8080/4121
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad9aaffa5c0c77080dcaeb2cdfa572dcbeeb1407
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] subprocess: allow Call() to read both stdout and stderr

2016-08-24 Thread Adar Dembo (Code Review)
Hello Dan Burkert, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4057

to look at the new patch set (#2).

Change subject: subprocess: allow Call() to read both stdout and stderr
..

subprocess: allow Call() to read both stdout and stderr

I'm going to use this in a new integration test for the tool.

Since the parent is now reading from two pipes, it needs to do so more
carefully. For example, if it read from stdout fully before looking at
stderr, both it and the child would deadlock if the child wrote 64k bytes
to the stderr pipe, hit the kernel limit, and got blocked.

I played around with an implementation based on poll(), but ultimately found
this one (based on libev) to be simpler.

Change-Id: If5f2be94c2e5cc0644a5bb2340adc4a71d844247
---
M src/kudu/util/CMakeLists.txt
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/subprocess.h
4 files changed, 163 insertions(+), 64 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/4057/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4057
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5f2be94c2e5cc0644a5bb2340adc4a71d844247
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] tool: basic integration test

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: tool: basic integration test
..


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/3058/

-- 
To view, visit http://gerrit.cloudera.org:8080/4058
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] tool: basic integration test

2016-08-24 Thread Adar Dembo (Code Review)
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4058

to look at the new patch set (#4).

Change subject: tool: basic integration test
..

tool: basic integration test

So far all it does is spot check some help pages, but in the future we
should augment it to test functionality too. For now that's not a big deal
because every tool function is covered in either master_migration-itest or
master_failover-itest.

Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa
---
M build-support/dist_test.py
M src/kudu/tools/CMakeLists.txt
A src/kudu/tools/kudu-tool-test.cc
M src/kudu/util/test_macros.h
4 files changed, 188 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/4058/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4058
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] tool: basic integration test

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: tool: basic integration test
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4058/3/build-support/dist_test.py
File build-support/dist_test.py:

Line 249: deps.extend(ldd_deps(d))
> do we need to worry about de-duping deps now?
Since all of the tests passed I can't tell whether we actually shipped 
duplicate dependencies, but I'll deduplicate here.


http://gerrit.cloudera.org:8080/#/c/4058/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

PS3, Line 70: expected_prefixes
> don't really understand the usage here
Replaced this with regexes, so it's a moot point now.


Line 91: // Strip away everything up to the usage string to test for 
prefixes.
> a little skeptical of this fancy verification vs just hard-coding a regex f
Done


Line 99: if (l.find(" " + m) != string::npos) {
> this isn't checking for a prefix
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4058
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib386882c1874e987d5824cfe742cc86627cd9eaa
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: Rolling-upgrades compat for changing TSRegistrationPB
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4062/4/src/kudu/master/ts_descriptor.cc
File src/kudu/master/ts_descriptor.cc:

PS4, Line 68:   using HostPortSet = std::unordered_set;
:   HostPortSet hostports;
Interesting, but why not just:

  unordered_set 
hostports;


PS4, Line 70:   for (int i = 0; i < pb1.size(); i++) {
: HostPort hp1;
: if (!HostPortFromPB(pb1.Get(i), ).ok()) return false;
: hostports.insert(hp1);
:   }
:   for (int i = 0; i < pb1.size(); i++) {
: HostPort hp2;
: if (!HostPortFromPB(pb2.Get(i), ).ok()) return false;
: if (!ContainsKey(hostports, hp2)) {
:   return false;
: }
:   }
You may be able to get away with unordered_sets of HostPortPB (maybe the 
generated code does the right thing):

  unordered_set hps1(pb1.begin(), pb1.end());
  unordered_set hps2(pb2.begin(), pb2.end());
  return hps1 == hps2;


-- 
To view, visit http://gerrit.cloudera.org:8080/4062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB

2016-08-24 Thread Mike Percy (Code Review)
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4062

to look at the new patch set (#4).

Change subject: Rolling-upgrades compat for changing TSRegistrationPB
..

Rolling-upgrades compat for changing TSRegistrationPB

This should allow for adding fields to TSRegistrationPB.
Only the host/port is checked.

Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
---
M src/kudu/master/master-test.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
4 files changed, 78 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/4062/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Actually support downgrade to version that has LocalConsensus

2016-08-24 Thread Mike Percy (Code Review)
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4059

to look at the new patch set (#3).

Change subject: Actually support downgrade to version that has LocalConsensus
..

Actually support downgrade to version that has LocalConsensus

Commit 74210b2546df9fd5dec7bb926eeb524362d2da90 attempted to support
downgrade from 0.10.x to 0.9.x however there is specific validation in
VerifyRaftConfig() in Kudu 0.9.x that requires the optional field
"local" to be set (checked using has_local()). So we must not rely on
the default, rather, we must always set that field.

This patch was manually tested as follows:

A 3-way replicated table was created in 0.10.0-RC1 plus this patch,
using a single master and 3 tablet servers. The services were killed,
then restarted using 0.9.1 binaries.

Change-Id: Icded76c6400b4802581671e2e41efb74a47c5ec4
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
2 files changed, 4 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/4059/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icded76c6400b4802581671e2e41efb74a47c5ec4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Rolling-upgrades compat for changing TSRegistrationPB

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Rolling-upgrades compat for changing TSRegistrationPB
..


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/3057/

-- 
To view, visit http://gerrit.cloudera.org:8080/4062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Actually support downgrade to version that has LocalConsensus

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Actually support downgrade to version that has LocalConsensus
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/3056/

-- 
To view, visit http://gerrit.cloudera.org:8080/4059
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icded76c6400b4802581671e2e41efb74a47c5ec4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Forward compat for changing TSRegistrationPB

2016-08-24 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: Forward compat for changing TSRegistrationPB
..


Patch Set 3:

(2 comments)

> How about a unit test showing a successful reregistration when
 > something other than the addresses has changed?

Done

http://gerrit.cloudera.org:8080/#/c/4062/3/src/kudu/master/ts_descriptor.cc
File src/kudu/master/ts_descriptor.cc:

Line 58: // Compares two repeated HostPortPB fields. Returns true if equal, 
false otherwise.
> oh, i see, yea that makes sense
Done


http://gerrit.cloudera.org:8080/#/c/4062/3/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:

Line 56: return port() == other.port() && host() == other.host();
> Any particular reason to compare the port before the host? I'd think the la
Comparing integers is cheaper.


-- 
To view, visit http://gerrit.cloudera.org:8080/4062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I794517d200b1c9ad74b3897b17aa808d68d7924b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 1:

Forgot to add: I was under the impression that after a client reconnects, the 
client2tablets entry is reused. I see now that's not the case at all.

-- 
To view, visit http://gerrit.cloudera.org:8080/4119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4119/1/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java:

Line 317: for (int port : tserverPorts) {
I don't think we're managing this list very well right now. We never actually 
remove entries from it.


-- 
To view, visit http://gerrit.cloudera.org:8080/4119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

2016-08-24 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Adar Dembo,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/4119

to review the following change.

Change subject: java: fix leak of TabletClient objects in client2tablets map
..

java: fix leak of TabletClient objects in client2tablets map

After running YCSB for a week, most of the clients had hit OOMEs. Some
heap dump analysis showed that the client2tablets map had hundreds of
thousands of leaked clients.

It seems that we were neglecting to remove the client from the
client2tablets map upon a disconnect. This fixes the issue and adds a
regression test which reproduced the bug.

Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.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/TestAsyncKuduSession.java
4 files changed, 52 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/4119/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3055/

-- 
To view, visit http://gerrit.cloudera.org:8080/4119
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] java: inherit from ASF parent pom

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: java: inherit from ASF parent pom
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4118/1/java/pom.xml
File java/pom.xml:

Line 34:   12
The latest version appears to be 18: 
https://repo.maven.apache.org/maven2/org/apache/apache/


-- 
To view, visit http://gerrit.cloudera.org:8080/4118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] java: inherit from ASF parent pom

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: java: inherit from ASF parent pom
..


Patch Set 1:

Oh sorry, just saw the associated email.

-- 
To view, visit http://gerrit.cloudera.org:8080/4118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] java: inherit from ASF parent pom

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: java: inherit from ASF parent pom
..


Patch Set 1:

I'm fine with this as long as we keep publishing snapshots every night or every 
commit.  Can we do that to the ASF repo?

-- 
To view, visit http://gerrit.cloudera.org:8080/4118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] java: inherit from ASF parent pom

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: java: inherit from ASF parent pom
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3054/

-- 
To view, visit http://gerrit.cloudera.org:8080/4118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] java: inherit from ASF parent pom

2016-08-24 Thread Todd Lipcon (Code Review)
Hello Dan Burkert,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/4118

to review the following change.

Change subject: java: inherit from ASF parent pom
..

java: inherit from ASF parent pom

This removes the Cloudera distribution management section and adds the
ASF parent pom to pick up the ASF nexus for distribution management.

Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011
---
M java/pom.xml
1 file changed, 8 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/4118/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf4e46b9ba50c74fb7943aabd2065609e3c5e011
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4100/6/python/kudu/tests/common.py
File python/kudu/tests/common.py:

PS6, Line 56: "--unlock_unsafe_flags",
: "--unlock_experimental_flags",
Nit: use single dash for consistency with the other flags. Below too.


http://gerrit.cloudera.org:8080/#/c/4100/6/src/kudu/util/flag_tags-test.cc
File src/kudu/util/flag_tags-test.cc:

Line 36: 
Nit: there's an extra empty line here.


Line 75: ASSERT_DEATH({ HandleCommonFlags();},
Nit: { HandleCommonFlags(); }

(space between semicolon and end brace).

Below too.


PS6, Line 82: StringVectorSink sink;
: ScopedRegisterSink reg();
You know you've made it as a project when you've got unit tests that check log 
output.


http://gerrit.cloudera.org:8080/#/c/4100/6/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:

Line 312: void CheckFlagsUnlocked() {
Nit: perhaps adjust the function name slightly? When I saw FooUnlocked() I 
assumed that meant a lock must be held when calling it.


-- 
To view, visit http://gerrit.cloudera.org:8080/4100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
..


Patch Set 21:

(7 comments)

Thank you for the review.  I addressed the issues you pointed at and will send 
the updated version with additional changes soon.

http://gerrit.cloudera.org:8080/#/c/3952/21//COMMIT_MSG
Commit Message:

PS21, Line 27: va(
> Interval
Done


http://gerrit.cloudera.org:8080/#/c/3952/21/src/kudu/client/batcher.h
File src/kudu/client/batcher.h:

PS21, Line 69: const scoped_refptr& error_collector,
 :   const client::sp::weak_ptr& session,
> both of these should be taken by value, and then moved into their field.
Done


http://gerrit.cloudera.org:8080/#/c/3952/21/src/kudu/client/client.h
File src/kudu/client/client.h:

PS21, Line 1143: when newly submitted
   :   ///   operations are about to overflow the buffer
> when the newly applied operation would overflow the buffer
Done


http://gerrit.cloudera.org:8080/#/c/3952/21/src/kudu/client/session-internal.cc
File src/kudu/client/session-internal.cc:

PS21, Line 428: requied
> required
Done


PS21, Line 451: Aplly
> Apply
Done


http://gerrit.cloudera.org:8080/#/c/3952/21/src/kudu/client/session-internal.h
File src/kudu/client/session-internal.h:

PS21, Line 86: apply write operation
> apply a write operation
Done


PS21, Line 86: Check whether it's possible
> Does this not actually apply the op?  If not I think the method should be r
Oops, exactly.  I messed up those comments, my bad.  Will fix.


-- 
To view, visit http://gerrit.cloudera.org:8080/3952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..


[c++-client] reduce in-flight-op partition key lifetime

This commit changes the C++ MetaCache to take the lookup partition key
by value, and updates Batcher to pass the partition key by move when
calling the MetaCache. Additionally, the partition key is no longer
stored as a field in the InFlightOp. The effect is that the InFlightOp
is smaller by the size of a std::string, and the lifetime of the
partition key is reduced from when the InFlightOp is complete to when
the meta cache lookup is complete.

Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Reviewed-on: http://gerrit.cloudera.org:8080/4114
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Adar Dembo 
---
M src/kudu/client/batcher.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
3 files changed, 9 insertions(+), 11 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Alexey Serbin: Looks good to me, but someone else must approve
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1500: Data race in 
RaftConsensusITest.TestCorruptReplicaMetadata
..


Patch Set 9:

Build Started http://104.196.14.100/job/kudu-gerrit/3053/

-- 
To view, visit http://gerrit.cloudera.org:8080/3823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata

2016-08-24 Thread Dinesh Bhat (Code Review)
Hello Dan Burkert, Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3823

to look at the new patch set (#9).

Change subject: KUDU-1500: Data race in 
RaftConsensusITest.TestCorruptReplicaMetadata
..

KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata

The test intends to corrupt the metadata of one of the tserver tablets.
While the cluster is in the process of resurrecting the failed tserver,
the Partition and PartitionSchema on that tserver is accessed using
TabletPeer in an unguarded manner via ListTablets RPCs.

The proposed fix here keeps the Partition and PartitionSchema immutable
once after it is loaded during replica bootstrap. These fields are never
overwritten again during Tablet Copy or any other workflow.
Also a unit test is added to exercise this data race window. i.e,
the unit test aims to issue ListTablets RPCs during a follower's
tablet copy stage and provides an optimistic coverage for the fix.

Testing: Passed about 2000 iteration of the failing test
raft_consensus-itest.TestCorruptReplicaMetadata

Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019
---
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/tablet/tablet_metadata.cc
3 files changed, 139 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/23/3823/9
-- 
To view, visit http://gerrit.cloudera.org:8080/3823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 6: Code-Review+1

-- 
To view, visit http://gerrit.cloudera.org:8080/4100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..


Patch Set 2: Code-Review+1

-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3052/

-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Dan Burkert (Code Review)
Hello Todd Lipcon, Alexey Serbin, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4114

to look at the new patch set (#2).

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..

[c++-client] reduce in-flight-op partition key lifetime

This commit changes the C++ MetaCache to take the lookup partition key
by value, and updates Batcher to pass the partition key by move when
calling the MetaCache. Additionally, the partition key is no longer
stored as a field in the InFlightOp. The effect is that the InFlightOp
is smaller by the size of a std::string, and the lifetime of the
partition key is reduced from when the InFlightOp is complete to when
the meta cache lookup is complete.

Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
---
M src/kudu/client/batcher.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
3 files changed, 9 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/4114/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 6:

Build Started http://104.196.14.100/job/kudu-gerrit/3051/

-- 
To view, visit http://gerrit.cloudera.org:8080/4100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-24 Thread Todd Lipcon (Code Review)
Hello Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4100

to look at the new patch set (#6).

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..

KUDU-1231. Add "unlock" flag for experimental and unsafe flags

This adds two new flags:
  --unlock_experimental_flags
  --unlock_unsafe_flags

If a flag is tagged as 'unsafe' or 'experimental', and the user tries
to set this flag on the command line without the corresponding 'unlock'
flag being set, then the process will exit at startup with an error.

Example error output without flags unlocked:
  E0824 14:04:57.263624 14821 flags.cc:296] Flag --never_fsync is unsafe and 
unsupported.
  E0824 14:04:57.263749 14821 flags.cc:302] 1 unsafe flag(s) in use.
  E0824 14:04:57.263761 14821 flags.cc:303] Use --unlock_unsafe_flags to 
proceed at your own risk.
  E0824 14:04:57.264104 14821 flags.cc:296] Flag 
--local_ip_for_outbound_sockets is experimental and unsupported.
  E0824 14:04:57.264128 14821 flags.cc:302] 1 experimental flag(s) in use.
  E0824 14:04:57.264137 14821 flags.cc:303] Use --unlock_experimental_flags to 
proceed at your own risk.
  

Example error output with flags unlocked:
  W0824 14:04:42.922560 14773 flags.cc:294] Enabled unsafe flag: 
--never_fsync=true
  W0824 14:04:42.923032 14773 flags.cc:294] Enabled experimental flag: 
--local_ip_for_outbound_sockets=127.0.0.1
  

Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
---
M docs/release_notes.adoc
M java/kudu-client/src/test/resources/flags
M python/kudu/tests/common.py
M src/kudu/client/client_samples-test.sh
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/util/flag_tags-test.cc
M src/kudu/util/flag_tags.h
M src/kudu/util/flags.cc
8 files changed, 137 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/4100/6
-- 
To view, visit http://gerrit.cloudera.org:8080/4100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 5:

Build Started http://104.196.14.100/job/kudu-gerrit/3050/

-- 
To view, visit http://gerrit.cloudera.org:8080/4100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-24 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4100/4/src/kudu/client/client_samples-test.sh
File src/kudu/client/client_samples-test.sh:

Line 125:   --unlock_experimental_flags \
> What about unsafe flags?  Not needed here?
I think this sample only uses an experimental flag 
(--local_ip_for_outbound_sockets)


-- 
To view, visit http://gerrit.cloudera.org:8080/4100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4114/1//COMMIT_MSG
Commit Message:

Line 9: This commit changes the C++ MetaCache to take the lookup partition key 
by value,
Nit: could you re-format the message a bit so the lines would be no longer than 
72 symbols?


-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] KuduSession: do not advertise thread-safety

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has submitted this change and it was merged.

Change subject: KuduSession: do not advertise thread-safety
..


KuduSession: do not advertise thread-safety

Do not advertise thread-safety of KuduSession methods for the
Kudu C++ client, even if they are thread-safe de facto.  This is
to have one set of semantics for both C++ and Java Kudu client
libraries (corresponding methods of the Java client are not
thread-safe).  Besides, current use cases for the Kudu client
assume that operations with KuduSession object do not involve
multiple threads.

Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd
Reviewed-on: http://gerrit.cloudera.org:8080/4105
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M docs/release_notes.adoc
M src/kudu/client/client.h
2 files changed, 4 insertions(+), 13 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4105
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Binglin Chang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KuduSession: do not advertise thread-safety

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: KuduSession: do not advertise thread-safety
..


Patch Set 3: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4105
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Binglin Chang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Replace incubator-kudu links with kudu in docs

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: Replace incubator-kudu links with kudu in docs
..


Patch Set 4: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..


Patch Set 1: Code-Review+1

-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-1157. Don't use array reference equality for EMPTY ARRAY

2016-08-24 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1157. Don't use array reference equality for EMPTY_ARRAY
..


KUDU-1157. Don't use array reference equality for EMPTY_ARRAY

It seems like the only way that the user can specify these byte[] bounds
is some deprecated 'raw bounds' APIs. So, this isn't likely a fix for
any real bugs. However, it's quite bad form to compare reference
equality on arrays.

Change-Id: I30163098926822aafbf23b03ba4c9e26a7c91349
Reviewed-on: http://gerrit.cloudera.org:8080/4101
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M 
java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
3 files changed, 14 insertions(+), 18 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4101
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I30163098926822aafbf23b03ba4c9e26a7c91349
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [docs] added tip on generic git commit guidelines

2016-08-24 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: [docs] added tip on generic git commit guidelines
..


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Replace incubator-kudu links with kudu in docs

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: Replace incubator-kudu links with kudu in docs
..


Patch Set 4: Code-Review+1

Thanks, this looks good to me!

P.S.  I would give it +2 but I don't have the privilege :)

-- 
To view, visit http://gerrit.cloudera.org:8080/4107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] [docs] added tip on generic git commit guidelines

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: [docs] added tip on generic git commit guidelines
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3049/

-- 
To view, visit http://gerrit.cloudera.org:8080/4115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] [docs] added tip on generic git commit guidelines

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4115

Change subject: [docs] added tip on generic git commit guidelines
..

[docs] added tip on generic git commit guidelines

Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927
---
M docs/contributing.adoc
1 file changed, 3 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/4115/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5493723f7f0b337b73e3ef5ed4a7621fe5249927
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 


[kudu-CR] Replace incubator-kudu links with kudu in docs

2016-08-24 Thread Mladen Kovacevic (Code Review)
Mladen Kovacevic has posted comments on this change.

Change subject: Replace incubator-kudu links with kudu in docs
..


Patch Set 4:

(3 comments)

Alex and Will, thanks for the comments. Let me know if anything else is needed 
here.

http://gerrit.cloudera.org:8080/#/c/4107/3//COMMIT_MSG
Commit Message:

Line 7: Replace incubator-kudu links with kudu in docs
> Could you re-format the commit message a bit to follow the git guideline:
Done


http://gerrit.cloudera.org:8080/#/c/4107/3/docs/contributing.adoc
File docs/contributing.adoc:

Line 52: git clone https://github.com/apache/kudu
> now that the repo name is kudu, I think this can be simplified to
Done


http://gerrit.cloudera.org:8080/#/c/4107/3/docs/installation.adoc
File docs/installation.adoc:

Line 242: $ git clone https://github.com/apache/kudu
> same with all these; the kudu at the end can be left off.
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] Replace incubator-kudu links with kudu in docs

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Replace incubator-kudu links with kudu in docs
..


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/3048/

-- 
To view, visit http://gerrit.cloudera.org:8080/4107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] Replace incubator-kudu links with kudu in docs

2016-08-24 Thread Mladen Kovacevic (Code Review)
Hello Will Berkeley, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4107

to look at the new patch set (#4).

Change subject: Replace incubator-kudu links with kudu in docs
..

Replace incubator-kudu links with kudu in docs

A number of docs were referring to the old incubator link:

  https://github.com/apache/incubator-kudu

As opposed to the new, non-incubator link.

  https://github.com/apache/kudu

We modify several of the documentation files to ensure that the
links are consistent, and up-to-date with the apache/kudu url.

Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
---
M RELEASING.adoc
M docs/contributing.adoc
M docs/developing.adoc
M docs/installation.adoc
M docs/release_notes.adoc
5 files changed, 17 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/4107/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] tests: add --stress cpu threads argument

2016-08-24 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged.

Change subject: tests: add --stress_cpu_threads argument
..


tests: add --stress_cpu_threads argument

This is equivalent to running 'stress -c ' except it's built into our
test binaries. This is quite handy for reproducing race conditions,
especially on dist-test where it isn't easy to just pop open another
terminal and run 'stress' at the same time.

Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425
Reviewed-on: http://gerrit.cloudera.org:8080/4106
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
Reviewed-by: Mike Percy 
---
M src/kudu/util/test_main.cc
1 file changed, 19 insertions(+), 0 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/4106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] tests: add --stress cpu threads argument

2016-08-24 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: tests: add --stress_cpu_threads argument
..


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No


[kudu-CR] KUDU-1231. Add "unlock" flag for experimental and unsafe flags

2016-08-24 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: KUDU-1231. Add "unlock" flag for experimental and unsafe flags
..


Patch Set 4: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4100/4/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:

PS4, Line 293: exit(1)
> Can you print out all enabled experimental and unsafe flags before exiting?
This is a good idea, and pretty easy to do.


-- 
To view, visit http://gerrit.cloudera.org:8080/4100
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec49e77fca604a7c5ee7501121a6263b7ee590d6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] Update contributing doc page with apache/kudu instead of apache/incubator-kudu

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: Update contributing doc page with apache/kudu instead of 
apache/incubator-kudu
..


Patch Set 3:

(1 comment)

Could you also update the RELEASING.adoc accordingly?

http://gerrit.cloudera.org:8080/#/c/4107/3//COMMIT_MSG
Commit Message:

Line 7: Update contributing doc page with apache/kudu instead
Could you re-format the commit message a bit to follow the git guideline:

https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines


-- 
To view, visit http://gerrit.cloudera.org:8080/4107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] Update contributing doc page with apache/kudu instead of apache/incubator-kudu

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: Update contributing doc page with apache/kudu instead of 
apache/incubator-kudu
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4107/3/docs/contributing.adoc
File docs/contributing.adoc:

Line 52: git clone https://github.com/apache/kudu.git kudu
now that the repo name is kudu, I think this can be simplified to

git clone https://github.com/apache/kudu


http://gerrit.cloudera.org:8080/#/c/4107/3/docs/installation.adoc
File docs/installation.adoc:

Line 242: $ git clone https://github.com/apache/kudu kudu
same with all these; the kudu at the end can be left off.


-- 
To view, visit http://gerrit.cloudera.org:8080/4107
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9b0560f073735e55243643ddada6eac0001b9d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mladen Kovacevic 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mladen Kovacevic 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Dan Burkert (Code Review)
Hello Adar Dembo, Todd Lipcon, Alexey Serbin,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/4114

to review the following change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..

[c++-client] reduce in-flight-op partition key lifetime

This commit changes the C++ MetaCache to take the lookup partition key by value,
and updates Batcher to pass the partition key by move when calling the 
MetaCache.
Additionally, the partition key is no longer stored as a field in the 
InFlightOp. The
effect is that the InFlightOp is smaller by the size of a std::string, and the
lifetime of the partition key is reduced from when the InFlightOp is complete to
when the meta cache lookup is complete.

Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
---
M src/kudu/client/batcher.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
3 files changed, 9 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/4114/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [c++-client] reduce in-flight-op partition key lifetime

2016-08-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: [c++-client] reduce in-flight-op partition key lifetime
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3047/

-- 
To view, visit http://gerrit.cloudera.org:8080/4114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia949b65a9447979c9c5eff324448af307a1db1b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] KUDU-1157. Don't use array reference equality for EMPTY ARRAY

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: KUDU-1157. Don't use array reference equality for EMPTY_ARRAY
..


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4101
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I30163098926822aafbf23b03ba4c9e26a7c91349
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] KuduSession: do not advertise thread-safety

2016-08-24 Thread Alexey Serbin (Code Review)
Hello Adar Dembo, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/4105

to look at the new patch set (#3).

Change subject: KuduSession: do not advertise thread-safety
..

KuduSession: do not advertise thread-safety

Do not advertise thread-safety of KuduSession methods for the
Kudu C++ client, even if they are thread-safe de facto.  This is
to have one set of semantics for both C++ and Java Kudu client
libraries (corresponding methods of the Java client are not
thread-safe).  Besides, current use cases for the Kudu client
assume that operations with KuduSession object do not involve
multiple threads.

Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd
---
M docs/release_notes.adoc
M src/kudu/client/client.h
2 files changed, 4 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/4105/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4105
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Binglin Chang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KuduSession: do not advertise thread-safety

2016-08-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: KuduSession: do not advertise thread-safety
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4105/2/src/kudu/client/client.h
File src/kudu/client/client.h:

PS2, Line 1011: /// @note This class is not thread-safe except where otherwise 
specified.
> Should this be removed too?
or change it to

@note This class is not thread-safe.


-- 
To view, visit http://gerrit.cloudera.org:8080/4105
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b3fcbd0d6446ccddce7a1812260c692eba18fcd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Binglin Chang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1534 : Added software version to ListMasters RPC

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: KUDU-1534 : Added software_version to ListMasters RPC
..


Patch Set 3:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/4099/3//COMMIT_MSG
Commit Message:

Line 7: KUDU-1534 : Added software_version to ListMasters RPC
Maybe add a note below about the cleanup you did?


http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:

PS3, Line 82: // Sent by the TS when it first heartbeats with a master. This 
sends the
: // master all of the necessary information about the current 
instance
: // of the TS. This is also used by master during master bringup.
The new comment describes how ServerRegistrationPB is used instead of what it 
means. The latter is more relevant here.


http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/integration-tests/cluster_itest_util.h
File src/kudu/integration-tests/cluster_itest_util.h:

Line 53: class ServerRegistrationPB;
Nit: sort alphabetically.


Line 78:   ServerRegistrationPB registration;
Hmm, how does the forward declaration help with this? The layout of 
ServerRegistrationPB must be known in order to satisfy this definition.


http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/master/master-path-handlers.h
File src/kudu/master/master-path-handlers.h:

Line 28: class ServerRegistrationPB;
Why is this forward declaration needed?


PS3, Line 68: MasterRegistrationPB
Does this type even exist? Does this method still need to be templated?


http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

Line 54: class ServerRegistrationPB;
Again, not seeing how this helps, given that to declare a ServerRegistrationPB 
on the stack you need the real class declaration.


PS3, Line 72: ServerRegistrationPB reg;
: master_->GetMasterRegistration();
: ASSERT_TRUE(reg.has_software_version());
This seems like an odd place to stash a test. Why not in its own TEST_F? Also, 
is this a better location than registration-test?


http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/master/master.h
File src/kudu/master/master.h:

Line 33: #include "kudu/common/wire_protocol.h"
Nit: sort alphabetically.


Line 41: class ServerRegistrationPB;
I don't think this helps given L135.


http://gerrit.cloudera.org:8080/#/c/4099/3/src/kudu/master/master.proto
File src/kudu/master/master.proto:

Line 26: 
Nit: why the added empty line?


Line 234:   optional ServerRegistrationPB registration = 2;
I wonder if this is backwards compatible. The message types are clearly 
different, but the layout of both is compatible. Can you find out?


Line 555: // TODO: Just use ServerRegistration here.
Nit: is this comment still relevant? Looks like we're providing a 
ServerRegistrationPB now.


-- 
To view, visit http://gerrit.cloudera.org:8080/4099
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I750eeb78c989daaa6fd5d9793b1218ae5993653c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] tests: add --stress cpu threads argument

2016-08-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: tests: add --stress_cpu_threads argument
..


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/4106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9214098a9f83fad78889b2ff9621b19109f92425
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No


[kudu-CR](branch-0.10.x) Update version to 0.10.1-SNAPSHOT on branch

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: Update version to 0.10.1-SNAPSHOT on branch
..


Patch Set 1:

I looked which files were changed for 0.6.0-SNAPSHOT, and it seems the 
following files are stuck with 0.6.0 version:

* java/kudu-csd/generate_mdl.py
* java/kudu-csd/src/descriptor/service.sdl

Is that OK?

Probably, it's related to 0.9.x --> 0.10.0 change as well.

-- 
To view, visit http://gerrit.cloudera.org:8080/4097
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8403d978605b5c5e385baa6ad6f7f4738b9c9f48
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-0.10.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] tool: improve help output

2016-08-24 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: tool: improve help output
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4038/1/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:

Line 96:   msg << " can be one of the following:\n";
> but then it would be lined up with the subcommands. Or you think I should i
I think that's perfectly fine, no need for the proposed indent.  The idea of an 
extra-spacing was inspired by valgrind and some other programs.

Moreover, this change has been already committed :)


-- 
To view, visit http://gerrit.cloudera.org:8080/4038
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0b4fc2da2b03edf8ce6b9998eeabd06a4fcd216d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes