[kudu-CR] Consolidate Row/CompactionInputRow printing on compaction

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: Consolidate Row/CompactionInputRow printing on compaction
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4988/3/src/kudu/tablet/compaction.cc
File src/kudu/tablet/compaction.cc:

PS3, Line 823: 3
Not a review comment per se, was it intentional to keep Input row loglevel 
different than output row loglevel (below) in this function ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd9f59094ac1f5f9c6343c1d5770dcc089f9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [java] Add checkstyle coverage to Java build

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: [java] Add checkstyle coverage to Java build
..


Patch Set 1:

> Build Failed
 > 
 > http://104.196.14.100/job/kudu-gerrit/4416/ : FAILURE

Failure looks like a flake? Not Java-related, anyway.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1578371509f7fcaaffa63bec0795bb3cf56c03c6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] [java] Add checkstyle coverage to Java build

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

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

Change subject: [java] Add checkstyle coverage to Java build
..

[java] Add checkstyle coverage to Java build

This patch introduces the Maven checkstyle plugin to Kudu. The
style rules are mostly Google style guide, with the notable
exception being a different include order. The include order
mirrors that of the C++ client and is described by the IntelliJ
rule

import static java.*

import static all other imports

import static org.apache.kudu.*

import java.*

import all other imports

import org.apache.kudu.*

As a sample, the recommendations of checkstyle have been
done in the flume sink.

Checkstyle warnings will not fail the build-- it remains
to be seen if that's a good idea.

Change-Id: I1578371509f7fcaaffa63bec0795bb3cf56c03c6
---
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduOperationsProducer.java
M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduOperationsProducer.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduOperationsProducer.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KuduSinkTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
A java/kudu_style.xml
M java/pom.xml
12 files changed, 378 insertions(+), 107 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1578371509f7fcaaffa63bec0795bb3cf56c03c6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 

[kudu-CR] Rename LogicalClock::NowForMetrics() to GetCurrentTime()

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: Rename LogicalClock::NowForMetrics() to GetCurrentTime()
..


Patch Set 3:

> Dinesh: Yeah I saw that and I pondered what you are suggesting.
 > Ended up not doing it because in the HybridClock::NowForMetrics is
 > really just used for metrics and does actually update the clocks's
 > logical value if needed. For a clock with a physical component this
 > makes sense: we want it to read the latest time. However for a
 > logical clock just getting the current time shouldn't change it.
 > 
 > It bummed me a little bit to break the symmetry, but since these
 > are not virtual methods I resigned to living with it :)

Sounds good, thank you for explaining, I was lazy enough to not to look into 
the definition of HybridClock::Now() earlier.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iac6f8dd9e79657b81409127e539e509da4b2b8de
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Reject CREATE TABLE ops with even replication factor

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Reject CREATE TABLE ops with even replication factor
..

Reject CREATE TABLE ops with even replication factor

Reject table creation with even replication factor,
and add an allow_unsafe_replication_factor
master flag to make it possible for advanced user.

Change-Id: I1674cc59cdfc2955d42fc5e4d8c0d962d9cc8e8e
---
M src/kudu/client/client-test.cc
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-admin-test.cc
7 files changed, 58 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1674cc59cdfc2955d42fc5e4d8c0d962d9cc8e8e
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Improve debuggability of the delta/compaction path

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Improve debuggability of the delta/compaction path
..


Patch Set 3:

(5 comments)

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

PS3, Line 12: changed
> nit: changes
Done


http://gerrit.cloudera.org:8080/#/c/4930/3/src/kudu/tablet/delta_compaction.cc
File src/kudu/tablet/delta_compaction.cc:

PS3, Line 151: compaction flush
> So which one is it? :)
Done


PS3, Line 173: " << "
> nit: remove
there, remove one ":" and replaced another one by "-"


http://gerrit.cloudera.org:8080/#/c/4930/3/src/kudu/tablet/delta_store.h
File src/kudu/tablet/delta_store.h:

PS3, Line 112: If 'pad' is true pads
> nit "is true, pads" or "is true it pads". I seem to be using commas a lot m
Done


PS3, Line 113: the result strings will compare as the delta keys themselves.
> nit: rephrase this, a bit terse. I'm not even 100% sure what it means.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc6c6ed76f5ab929c04410228d5ea70f4fc9660
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Improve debuggability of the delta/compaction path

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Improve debuggability of the delta/compaction path
..

Improve debuggability of the delta/compaction path

This adds/modifies some log statements on the delta/compaction path
to aid with debugging. For example it makes some log statements
in delta_compaction.cc output the row id instead of the row index
within the block, which helps when grepping for changes to a particular
row.

Change-Id: I0fc6c6ed76f5ab929c04410228d5ea70f4fc9660
---
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile.cc
4 files changed, 29 insertions(+), 13 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0fc6c6ed76f5ab929c04410228d5ea70f4fc9660
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Consolidate Row/CompactionInputRow printing on compaction

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Consolidate Row/CompactionInputRow printing on compaction
..

Consolidate Row/CompactionInputRow printing on compaction

We are duplicating row printing code in a lot of places. This adds
a couple of methods to avoid that.

Change-Id: Ifd9f59094ac1f5f9c6343c1d5770dcc089f9
---
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/delta_compaction.cc
3 files changed, 23 insertions(+), 28 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifd9f59094ac1f5f9c6343c1d5770dcc089f9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a macro to LOG and return on a non-OK status

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a macro to LOG and return on a non-OK status
..

Add a macro to LOG and return on a non-OK status

We often see the following pattern:

Status s = someOperation();

if (!s.ok()) {
  LOG(ERROR) << "A critical error occurred at blah. Status: " << s.ToString();
  return s;
}

This is cumbersome. This macro allows to do the same the following way:

RETURN_NOT_OK_LOG(ERROR, someOperation(), "A critical error occurred at blah.");

The macro also prints the status (though at the beginning of the
message and not at the end so that we can have arbirarily long
debugging information but the status is easy to find).

Change-Id: Ibb906e7240d7a289a7822444bc9cbf748a555efb
---
M src/kudu/util/status.h
1 file changed, 10 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb906e7240d7a289a7822444bc9cbf748a555efb
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [doc] 1.1.0 release notes for tools

[Dinesh Bhat (Code Review)]

Hello Jean-Daniel Cryans, Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [doc] 1.1.0 release notes for tools
..

[doc] 1.1.0 release notes for tools

Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
---
M docs/release_notes.adoc
1 file changed, 7 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [doc] 1.1.0 release notes for tools

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: [doc] 1.1.0 release notes for tools
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5012/3/docs/release_notes.adoc
File docs/release_notes.adoc:

PS3, Line 57: tablet
> replica
Done


PS3, Line 59: delete the
:   replica of a tablet
> "delete a replica"
Done


PS3, Line 60: when the tablet server is unable to restart due to bad state of 
the replica.
> Is this part really necessary? "kudu local_replica delete" can be used for 
Yeah, I wanted to emphasize implicitly that this tool is mainly used when 
tablet server is offline for so and so reasons. Come to think of it, the fact 
that it's an action under 'local_replica' makes it obvious that this tool can 
be used only when the server is offline, isn't it ? Removed it now.

Also, this tool only works with --clean_unsafe flag for the deletion to happen. 
otherwise it throws 'currently unsupported' and that support for 'tombstoning 
the tablet when server is offline' will be part of upcoming release. Although 
all this has clear help description under the tool, should we put that in 
release notes as well ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Workaround test failures running with MIT krb5 1.10
..


Patch Set 5:

> > But now we're stretching ourselves to accommodate el6's libkrb5.
 > Does libkrb5 also receive a high number of updates? If not, I'd
 > argue we should ship it ourselves and use a known good version.
 > 
 > I don't know what classifies as a high number, but looking at the
 > Debian changelog for Ubuntu 14.04, I see 18 CVEs in there (12 from
 > 2014, 6 from 2015). Looking at a RHEL 6.6 system, I see 4 releases
 > in 2015 and a bunch from 2014 as well. So, empirically, this
 > library does get updated fairly frequently by distro vendors, and I
 > can imagine that a security-conscious admin would be nervous about
 > taking an embedded version.
 
I think that does qualify as high, actually. I was hoping for a very low 
number, perhaps even 0.

 > Also keep in mind that this stuff is used by the client library,
 > and having our client try to static-link in its own krb5 and sasl
 > seems like a dangerous proposition since it is very likely to be
 > running embedded in another process which is also using krb5/sasl
 > (and where we actively want to be picking up credentials kinitted
 > by the embedder, etc).

Why? I would expect that credential passing between the embedder and the client 
will be done explicitly (via client API calls) and abstractly enough that the 
fact that the client uses MIT krb5 is just an implementation detail. Is that 
not going to be the case? I'm not a fan of dependencies dictating our client 
APIs, to the point where we've even discussed a C API to elide STL dependencies.

Anyway, if this interface is abstract, the client's use of symbol hiding should 
prevent the embedder from caring about the krb5/sasl usage of the client, 
regardless of its linkage.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
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-HasComments: No

[kudu-CR] Consolidate Row/CompactionInputRow printing on compaction

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Consolidate Row/CompactionInputRow printing on compaction
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4988/2/src/kudu/tablet/compaction.h
File src/kudu/tablet/compaction.h:

PS2, Line 236: CompactionInputToString
> CompactionInput*Row*ToString?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd9f59094ac1f5f9c6343c1d5770dcc089f9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [doc] updated release notes for Kudu C++ client
..


[doc] updated release notes for Kudu C++ client

Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Reviewed-on: http://gerrit.cloudera.org:8080/5006
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M docs/release_notes.adoc
1 file changed, 9 insertions(+), 2 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Workaround test failures running with MIT krb5 1.10
..


Patch Set 5:

> But now we're stretching ourselves to accommodate el6's libkrb5. Does libkrb5 
> also receive a high number of updates? If not, I'd argue we should ship it 
> ourselves and use a known good version.

I don't know what classifies as a high number, but looking at the Debian 
changelog for Ubuntu 14.04, I see 18 CVEs in there (12 from 2014, 6 from 2015). 
Looking at a RHEL 6.6 system, I see 4 releases in 2015 and a bunch from 2014 as 
well. So, empirically, this library does get updated fairly frequently by 
distro vendors, and I can imagine that a security-conscious admin would be 
nervous about taking an embedded version.


>I asked Dan about this and he said that due to how libsasl uses dlopen() to 
>get to libkrb5, it's really hard (or maybe impossible) to get it to prefer a 
>different version. Is that something you've looked into? Is there some way to 
>customize it?
If it's impossible, we'd have to ship libsasl too, and patch it to load libkrb5 
differently. I don't think that'd be the worst thing in the world (provided 
libsasl, like libkrb5 and unlike openssl, does not receive updates), but it'd 
be a lot more work than what you're doing here.

Yea, this stuff gets nasty, too, because once we're shipping libsasl, we also 
need to make sure that libsasl loads our own sasl modules, and not the ones 
from the system SASL path. We went down this route before but reversed it in 
152ff259de7ea44c81e17c59fd4a7c5f41ba712e a couple years ago, and I recall it 
was a big pain in the butt.

Also keep in mind that this stuff is used by the client library, and having our 
client try to static-link in its own krb5 and sasl seems like a dangerous 
proposition since it is very likely to be running embedded in another process 
which is also using krb5/sasl (and where we actively want to be picking up 
credentials kinitted by the embedder, etc).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
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-HasComments: No

[kudu-CR] Allow binaries built on el6.6+ to run on el6.4

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Allow binaries built on el6.6+ to run on el6.4
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5011/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

PS1, Line 259: CentOS|Red Hat Enterprise
> good question. Do you happen to know of an OEL system I can ssh into and po
I don't have one, no. Given that we don't explicitly advertise OEL support, 
leaving it as-is is fine with me.


http://gerrit.cloudera.org:8080/#/c/5011/1/thirdparty/vars.sh
File thirdparty/vars.sh:

Line 173: OPENSSL_WORKAROUND_DIR="$TP_DIR/installed/openssl-el6-workaround"
> I figured since it's a "weird" install (i.e it has a whole /usr directory i
Makes sense.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I115540d59580170404f9e65303214edf95b592e6
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-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [doc] updated release notes for Kudu C++ client
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Add a macro to LOG and return on a non-OK status

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a macro to LOG and return on a non-OK status
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4927/4/src/kudu/util/status.h
File src/kudu/util/status.h:

PS4, Line 66: level, s, msg
> nit: it would read better for me to put the log level before the message, n
was just trying to keep this consistent with the one above, but I prefer your 
suggestion. Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb906e7240d7a289a7822444bc9cbf748a555efb
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Workaround test failures running with MIT krb5 1.10
..


Patch Set 5:

(6 comments)

It makes sense to stretch ourselves to accommodate various openssl versions, 
because we don't want to ship our own openssl due to the high number of updates 
that it gets.

But now we're stretching ourselves to accommodate el6's libkrb5. Does libkrb5 
also receive a high number of updates? If not, I'd argue we should ship it 
ourselves and use a known good version.

I asked Dan about this and he said that due to how libsasl uses dlopen() to get 
to libkrb5, it's really hard (or maybe impossible) to get it to prefer a 
different version. Is that something you've looked into? Is there some way to 
customize it?

If it's impossible, we'd have to ship libsasl too, and patch it to load libkrb5 
differently. I don't think that'd be the worst thing in the world (provided 
libsasl, like libkrb5 and unlike openssl, does not receive updates), but it'd 
be a lot more work than what you're doing here.

Anyway, I want to at least have this conversation, if only so I'll understand 
how many CVEs and the like libkrb5/libsasl receive.

http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/CMakeLists.txt
File src/kudu/security/CMakeLists.txt:

PS5, Line 23: krb5
Can you add a find_library(REQUIRED) for this in the main CMakeLists.txt?


http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/init.cc
File src/kudu/security/init.cc:

Line 109: 
I'm kind of surprised you need to "sanitize" these environment variables given 
that we're calling into the krb5 library directly here. I would expect that the 
env variables have an effect on the krb5 CLI tools, but surprised that they 
would on direct library calls.


http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/test/krb5_realm_override.cc
File src/kudu/security/test/krb5_realm_override.cc:

PS5, Line 74:   CHECK(g_orig_krb5_get_host_realm);
:   CHECK(g_orig_krb5_get_default_realm);
:   CHECK(g_orig_krb5_free_default_realm);
Maybe have these return the right krb5_error_code on failure? Then you could 
remove the glog dependency and keep this code more "pristine" (i.e. less 
Kudu-dependent).


http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/util/test_main.cc
File src/kudu/util/test_main.cc:

Line 20: #include 
What's this for?


Line 24: #include 
And this?


Line 39: using std::string;
And this.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
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-HasComments: Yes

[kudu-CR] Allow binaries built on el6.6+ to run on el6.4

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Allow binaries built on el6.6+ to run on el6.4
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5011/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

PS1, Line 259: CentOS|Red Hat Enterprise
> What about Oracle Enterprise Linux? Isn't its userspace binary compatible w
good question. Do you happen to know of an OEL system I can ssh into and poke 
around? If not, I'd rather leave as is until someone is building on that system 
and runs into this issue.


http://gerrit.cloudera.org:8080/#/c/5011/1/thirdparty/vars.sh
File thirdparty/vars.sh:

Line 173: OPENSSL_WORKAROUND_DIR="$TP_DIR/installed/openssl-el6-workaround"
> Why not installed/uninstrumented/... ?
I figured since it's a "weird" install (i.e it has a whole /usr directory 
inside it) it doesn't make sense to overlay on top of the normal "install" 
directory.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I115540d59580170404f9e65303214edf95b592e6
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-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] webserver: improve SSL certificate handling

[Todd Lipcon (Code Review)]

Hello Dan Burkert,

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

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

to review the following change.

Change subject: webserver: improve SSL certificate handling
..

webserver: improve SSL certificate handling

* Allows users to specify a separate location for PEM private-key file
  using --webserver_private_key_file (previously required private-key
  and cert. to be in same file).

* Allows users to specify a shell command to run to get the password
  for the webserver's private-key file using
  --webserver_private_key_password_cmd

The separate configuration of cert and key is apparently more commonly
used, so this simplifies deployment. The use of a script to provide the
password for the private key makes it easier to integrate with external
credential providers.

In order to test this, I reconfigured our thirdparty curl build to include
HTTPS support.

This mirrors the change made in Impala by IMPALA-2051 (8bbe45393c7).

Change-Id: I4b508cebbe6f31556e6d5a5fba5e5e9fb44cf1b9
---
M src/kudu/rpc/rpc-test-base.h
M src/kudu/security/CMakeLists.txt
A src/kudu/security/test/test_certs.cc
A src/kudu/security/test/test_certs.h
M src/kudu/server/CMakeLists.txt
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver_options.cc
M src/kudu/server/webserver_options.h
M src/kudu/util/curl_util.cc
M src/kudu/util/curl_util.h
M thirdparty/build-definitions.sh
12 files changed, 295 insertions(+), 22 deletions(-)


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

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

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [doc] updated release notes for Kudu C++ client
..

[doc] updated release notes for Kudu C++ client

Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
---
M docs/release_notes.adoc
1 file changed, 9 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [doc] updated release notes for Kudu C++ client
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5006/3/docs/release_notes.adoc
File docs/release_notes.adoc:

PS3, Line 70:   
> extra space here
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Unbreak the build: Pull RowChangelistEncoder::get type()

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Unbreak the build: Pull RowChangelistEncoder::get_type()
..


Unbreak the build: Pull RowChangelistEncoder::get_type()

Misordered push of 3c68deacc05f6b5a9b82522baa38e046744b1815
caused the build to fail. This just pulls that method in.

Change-Id: If83f7d26ecc5632b6645b1aec927580f202f7d85
Reviewed-on: http://gerrit.cloudera.org:8080/5013
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Kudu Jenkins
---
M src/kudu/common/row_changelist.h
1 file changed, 4 insertions(+), 0 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If83f7d26ecc5632b6645b1aec927580f202f7d85
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Workaround test failures running with MIT krb5 1.10
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/init.cc
File src/kudu/security/init.cc:

PS5, Line 88: krb5_creds creds
> ah, good catch, I think you're right. Wonder why LSAN didn't catch this
Interesting, indeed.  May be, this is the case when the creds do not contain 
much at all?


PS5, Line 112: "KRB5CCNAME"
Does it make sense to introduce constants for those Kerberos-related env 
variables?  It seems we have those string literals in multiple places now.  At 
least, it could help to avoid mistakes related to typos/etc.


http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/test/krb5_realm_override.cc
File src/kudu/security/test/krb5_realm_override.cc:

PS5, Line 78: int
nit: they have krb5_error_code type for that, if you wish.


PS5, Line 92: ret_realms
nit: probably it's too paranoid, but does it make sense to check for non-null 
result of malloc(), and, may be, strdup() as well (since they are plain C 
functions and don't throw std::bad_alloc if no memory available)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Allow binaries built on el6.6+ to run on el6.4

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Allow binaries built on el6.6+ to run on el6.4
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5011/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

PS1, Line 259: CentOS|Red Hat Enterprise
What about Oracle Enterprise Linux? Isn't its userspace binary compatible with 
RHEL?


http://gerrit.cloudera.org:8080/#/c/5011/1/thirdparty/install-openssl-el6-workaround.sh
File thirdparty/install-openssl-el6-workaround.sh:

PS1, Line 39: RhEL
RHEL


http://gerrit.cloudera.org:8080/#/c/5011/1/thirdparty/vars.sh
File thirdparty/vars.sh:

Line 173: OPENSSL_WORKAROUND_DIR="$TP_DIR/installed/openssl-el6-workaround"
Why not installed/uninstrumented/... ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I115540d59580170404f9e65303214edf95b592e6
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] [doc] 1.1.0 release notes for tools

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [doc] 1.1.0 release notes for tools
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5012/3/docs/release_notes.adoc
File docs/release_notes.adoc:

PS3, Line 57: tablet
replica


PS3, Line 59: delete the
:   replica of a tablet
"delete a replica"


PS3, Line 60: when the tablet server is unable to restart due to bad state of 
the replica.
Is this part really necessary? "kudu local_replica delete" can be used for 
various reasons; why bother calling out one? Especially since you didn't call 
out any reasons for the other new tools.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [doc] updated release notes for Kudu C++ client
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5006/3/docs/release_notes.adoc
File docs/release_notes.adoc:

PS3, Line 70:   
extra space here


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [doc] 1.1.0 release notes for tools

[Dinesh Bhat (Code Review)]

Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: [doc] 1.1.0 release notes for tools
..

[doc] 1.1.0 release notes for tools

Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
---
M docs/release_notes.adoc
1 file changed, 8 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Unbreak the build: Pull RowChangelistEncoder::get type()

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new patch set (#2).

Change subject: Unbreak the build: Pull RowChangelistEncoder::get_type()
..

Unbreak the build: Pull RowChangelistEncoder::get_type()

Misordered push of 3c68deacc05f6b5a9b82522baa38e046744b1815
caused the build to fail. This just pulls that method in.

Change-Id: If83f7d26ecc5632b6645b1aec927580f202f7d85
---
M src/kudu/common/row_changelist.h
1 file changed, 4 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If83f7d26ecc5632b6645b1aec927580f202f7d85
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [doc] 1.1.0 release notes for tools

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: [doc] 1.1.0 release notes for tools
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5012/2/docs/release_notes.adoc
File docs/release_notes.adoc:

PS2, Line 59: recovery
Seems like the wrong word considering we're deleting data... maybe "failure 
recovery" tool would be more accurate?


PS2, Line 60: was not able to run
"is unable to restart"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] ubsan: use our own libstdcxx

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: ubsan: use our own libstdcxx
..


Patch Set 2:

> I don't think we need this anymore, right? Or if we want to do it,
 > we should probably just use the libc++ that we are using for tsan?

Right, we should use libc++ instead. But to do that effectively we'll also need 
to rejigger the thirdparty build a bit, to build libc++ in all configurations, 
not just 'tsan'.

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

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

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [doc] updated release notes for Kudu C++ client
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5006/2/docs/release_notes.adoc
File docs/release_notes.adoc:

Line 62: 
> Nit: the three bullet points refer to a client as either a " clie
Done.  I think your choice is the best.


PS2, Line 74: or invocation of the callback passed into 
KuduSession::FlushAsync()
> Nit: "or, if using asynchronous flushing, since last invocation of the call
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [doc] updated release notes for Kudu C++ client
..

[doc] updated release notes for Kudu C++ client

Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
---
M docs/release_notes.adoc
1 file changed, 9 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [doc] 1.1.0 release notes for tools

[Dinesh Bhat (Code Review)]

Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: [doc] 1.1.0 release notes for tools
..

[doc] 1.1.0 release notes for tools

Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
---
M docs/release_notes.adoc
1 file changed, 8 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [doc] 1.1.0 release notes for tools

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: [doc] 1.1.0 release notes for tools
..


Patch Set 1:

(2 comments)

I also added couple of more tools which got pushed few minutes ago.

http://gerrit.cloudera.org:8080/#/c/5012/1/docs/release_notes.adoc
File docs/release_notes.adoc:

Line 153: === Command line tools
> Wrong section, this is known issues and limitations. You want to be all the
Done, added just after 'optimizations and improvements' section.


PS1, Line 155: is added
> That feels wrong. Reading through http://ell.stackexchange.com/questions/17
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take 
a RowChangeListEncoder as an out param
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4928/5//COMMIT_MSG
Commit Message:

PS5, Line 10: insted
nit: instead


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Rename LogicalClock::NowForMetrics() to GetCurrentTime()

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Rename LogicalClock::NowForMetrics() to GetCurrentTime()
..


Patch Set 2:

Dinesh: Yeah I saw that and I pondered what you are suggesting. Ended up not 
doing it because in the HybridClock::NowForMetrics is really just used for 
metrics and does actually update the clocks's logical value if needed. For a 
clock with a physical component this makes sense: we want it to read the latest 
time. However for a logical clock just getting the current time shouldn't 
change it.

It bummed me a little bit to break the symmetry, but since these are not 
virtual methods I resigned to living with it :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iac6f8dd9e79657b81409127e539e509da4b2b8de
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Improve debuggability of the delta/compaction path

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Improve debuggability of the delta/compaction path
..


Patch Set 3:

(5 comments)

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

PS3, Line 12: changed
nit: changes


http://gerrit.cloudera.org:8080/#/c/4930/3/src/kudu/tablet/delta_compaction.cc
File src/kudu/tablet/delta_compaction.cc:

PS3, Line 151: compaction flush
So which one is it? :)


PS3, Line 173: " << "
nit: remove

Also it's a lot of ":" in that line.


http://gerrit.cloudera.org:8080/#/c/4930/3/src/kudu/tablet/delta_store.h
File src/kudu/tablet/delta_store.h:

PS3, Line 112: If 'pad' is true pads
nit "is true, pads" or "is true it pads". I seem to be using commas a lot more 
than you do.


PS3, Line 113: the result strings will compare as the delta keys themselves.
nit: rephrase this, a bit terse. I'm not even 100% sure what it means.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc6c6ed76f5ab929c04410228d5ea70f4fc9660
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Consolidate Row/CompactionInputRow printing on compaction

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Consolidate Row/CompactionInputRow printing on compaction
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4988/2/src/kudu/tablet/compaction.h
File src/kudu/tablet/compaction.h:

PS2, Line 236: CompactionInputToString
CompactionInput*Row*ToString?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd9f59094ac1f5f9c6343c1d5770dcc089f9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Don't do UNDO garbage collection until after the REDO->UNDO transformation

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Don't do UNDO garbage collection until after the REDO->UNDO 
transformation
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4993/2/src/kudu/tablet/compaction.cc
File src/kudu/tablet/compaction.cc:

Line 619: // If the row has a DELETE redo which is older than ahm, 
is_garbage_collected is set to true.
Shouldn't this be in the header file? I also found this comment confusing, just 
reading this code without a lot of context you'd expect a variable named like 
that would imply that garbage collection took place, but we just run a check 
(which could be done outside of this function or in a different function).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I10e4bbb1cac68474756ea1716cfc4c5420cf503e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Workaround test failures running with MIT krb5 1.10
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/init.cc
File src/kudu/security/init.cc:

PS5, Line 88: krb5_creds creds
> What about calling krb5_free_cred_contents() on creds for clean up?
ah, good catch, I think you're right. Wonder why LSAN didn't catch this


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a macro to LOG and return on a non-OK status

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a macro to LOG and return on a non-OK status
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4927/4/src/kudu/util/status.h
File src/kudu/util/status.h:

PS4, Line 66: level, s, msg
nit: it would read better for me to put the log level before the message, no?

KUDU_RETURN_NOT_OK(Foo(), WARNING, "blah")?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb906e7240d7a289a7822444bc9cbf748a555efb
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Release notes for the Java client in 1.1

[Jean-Daniel Cryans (Code Review)]

Hello Adar Dembo, John Russell, Kudu Jenkins,

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

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

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

Change subject: Release notes for the Java client in 1.1
..

Release notes for the Java client in 1.1

Change-Id: Idf04d1db4769e19e6421628bbe980cb4c058c62c
---
M docs/release_notes.adoc
1 file changed, 22 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf04d1db4769e19e6421628bbe980cb4c058c62c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [tools] Manual recovery tools (part 1)

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [tools] Manual recovery tools (part 1)
..


[tools] Manual recovery tools (part 1)

This change introduces two recovery tools:
1) 'kudu remote_replica copy' copies the given tablet onto
   a destination server irrespective of the state of the
   destination replica (--force option provided).
   This is a port from https://gerrit.cloudera.org/#/c/3582/
2) 'kudu local_replica delete' (KUDU-1618) deletes a local replica
   of a tablet when the tablet server can not come up due to a
   bad replica of a tablet. This tool should be used with caution,
   hence added a --clean_unsafe flag to warn the user with
   consequences. As of this change, tool supports
   deleting the tablet permanently if used with --clean_unsafe flag,
   and fails with 'unsupported action error' without the flag.
   A future patch will add a default action of tombstoning
   the tablet instead of permanently deleting it.

Also added few tests exercising typical usage scenario for these tools.
Added trace to stderr under existing RunAction routines to throw more
info in the log if an action fails.

Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Reviewed-on: http://gerrit.cloudera.org:8080/4834
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
11 files changed, 456 insertions(+), 65 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Release notes for the Java client in 1.1

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Release notes for the Java client in 1.1
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5002/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 59: currently
:   available
> You didn't drop "currently".
Missed the highlighting on the previous line.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf04d1db4769e19e6421628bbe980cb4c058c62c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Workaround test failures running with MIT krb5 1.10
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4990/5/src/kudu/security/init.cc
File src/kudu/security/init.cc:

PS5, Line 88: krb5_creds creds
What about calling krb5_free_cred_contents() on creds for clean up?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] ubsan: use our own libstdcxx

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: ubsan: use our own libstdcxx
..


Patch Set 2:

I don't think we need this anymore, right? Or if we want to do it, we should 
probably just use the libc++ that we are using for tsan?

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

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

[kudu-CR] Account for REINSERTs in delta stats

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Account for REINSERTs in delta stats
..


Account for REINSERTs in delta stats

This makes DeltaStats also account for REINSERTs. This is not
actually used outside of tests, but seems like it would be
silly to not count this type of delta. In the future it might
be useful for selecting undo deltas for minor delta compaction.

The protobuf field is optional to preserve data format
compatibility.

Change-Id: Idd60f6c1c12803d339f5f8d96c6b089fab21b13f
Reviewed-on: http://gerrit.cloudera.org:8080/4932
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/tablet/delta_stats.cc
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/tablet.proto
3 files changed, 46 insertions(+), 15 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Idd60f6c1c12803d339f5f8d96c6b089fab21b13f
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Enforce that REINSERTs are not supported in DeltaMemStores

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Enforce that REINSERTs are not supported in DeltaMemStores
..


Enforce that REINSERTs are not supported in DeltaMemStores

We never really supported REINSERTs in the delta memstore but had
code that seemed like we eventually would. This adds a CHECK to
make sure we never see REINSERTs in the DeltaMemStore and also
simplifies some of the code accordingly.

Change-Id: I04d0ea70969dcf60b3690d21122f3a9d497c29e3
Reviewed-on: http://gerrit.cloudera.org:8080/4991
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/tablet/deltamemstore.cc
M src/kudu/tablet/deltamemstore.h
2 files changed, 9 insertions(+), 17 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I04d0ea70969dcf60b3690d21122f3a9d497c29e3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add IN LIST release notes

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Add IN LIST release notes
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I61a6a1871098a3252e09556ab510ca9693d14d74
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Account for REINSERTs in delta stats

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Account for REINSERTs in delta stats
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd60f6c1c12803d339f5f8d96c6b089fab21b13f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add IN LIST release notes

[Dan Burkert (Code Review)]

Dan Burkert has uploaded a new patch set (#2).

Change subject: Add IN LIST release notes
..

Add IN LIST release notes

Change-Id: I61a6a1871098a3252e09556ab510ca9693d14d74
---
M docs/release_notes.adoc
1 file changed, 4 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I61a6a1871098a3252e09556ab510ca9693d14d74
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Add IN LIST release notes

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Add IN LIST release notes
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5010/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 50: IN LIST predicate pushdown support
> Looking at previous release notes, we don't do statements like this. More c
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I61a6a1871098a3252e09556ab510ca9693d14d74
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Enforce that REINSERTs are not supported in DeltaMemStores

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Enforce that REINSERTs are not supported in DeltaMemStores
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I04d0ea70969dcf60b3690d21122f3a9d497c29e3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [tools] Manual recovery tools (part 1)

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 17: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add IN LIST release notes

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Add IN LIST release notes
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5010/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 50: IN LIST predicate pushdown support
Looking at previous release notes, we don't do statements like this. More 
common would be "IN LIST predicate pushdown support was added to allow 
optimized..." or something like that.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I61a6a1871098a3252e09556ab510ca9693d14d74
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Workaround test failures running with MIT krb5 1.10
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [doc] 1.1.0 release notes for tools

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: [doc] 1.1.0 release notes for tools
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5012/1/docs/release_notes.adoc
File docs/release_notes.adoc:

Line 153: === Command line tools
Wrong section, this is known issues and limitations. You want to be all the way 
up.


PS1, Line 155: is added
That feels wrong. Reading through 
http://ell.stackexchange.com/questions/17132/difference-between-is-added-and-was-added
 I'd write "has been added" or "was added".


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Workaround test failures running with MIT krb5 1.10
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4990/4/src/kudu/security/test/krb5_realm_override.cc
File src/kudu/security/test/krb5_realm_override.cc:

Line 74:   CHECK(g_orig_krb5_get_host_realm)
> Is there any way get_host_realm would be found, but not get_default_realm o
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] macOS: fix compile error introduced by 141de3377e6bdd

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: macOS: fix compile error introduced by 141de3377e6bdd
..


macOS: fix compile error introduced by 141de3377e6bdd

Change-Id: Id2a9556810feeb3128b97d60afb18a2547e0af5c
Reviewed-on: http://gerrit.cloudera.org:8080/5009
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
1 file changed, 6 insertions(+), 2 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

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

[kudu-CR] Workaround test failures running with MIT krb5 1.10

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Workaround test failures running with MIT krb5 1.10
..

Workaround test failures running with MIT krb5 1.10

This fixes a variety of issues seen when running on el6, which has MIT
krb5 1.10.

* This version doesn't support KRB5_CLIENT_KTNAME. So, we need to
  explicitly login servers from their provided keytabs when the server
  starts. This ensures that servers have proper client credentials in
  order to send kerberized RPCs to other servers.

* In our minicluster, we use strange local IPs like '127.x.y.z' for
  testing. These don't have any associated hostnames in reverse DNS. MIT
  krb5 has a bug (or lack of feature) whereby it cannot handle service
  principals whose hostname component is a numeric IP rather than a
  hostname. Specifically, it fails to determine the Kerberos Realm for a
  principal with a numeric host.

  This patch works around the issue by overriding the
  krb5_get_host_realm() function using a combination of LD_PRELOAD and
  statically including this symbol in our tests. See the comments at the
  top of krb5_realm_override.cc for details.

With this patch, external_mini_cluster-test passes on el6 whereas it
used to fail.

Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/security/CMakeLists.txt
A src/kudu/security/init.cc
A src/kudu/security/init.h
A src/kudu/security/test/krb5_realm_override.cc
M src/kudu/server/CMakeLists.txt
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_main.cc
11 files changed, 304 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] Release notes for the Java client in 1.1

[Jean-Daniel Cryans (Code Review)]

Hello Adar Dembo, John Russell, Kudu Jenkins,

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

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

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

Change subject: Release notes for the Java client in 1.1
..

Release notes for the Java client in 1.1

Change-Id: Idf04d1db4769e19e6421628bbe980cb4c058c62c
---
M docs/release_notes.adoc
1 file changed, 22 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf04d1db4769e19e6421628bbe980cb4c058c62c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Allow binaries built on el6.6+ to run on el6.4

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Allow binaries built on el6.6+ to run on el6.4
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I115540d59580170404f9e65303214edf95b592e6
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] [doc] 1.1.0 release notes for tools

[Dinesh Bhat (Code Review)]

Hello Jean-Daniel Cryans,

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

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

to review the following change.

Change subject: [doc] 1.1.0 release notes for tools
..

[doc] 1.1.0 release notes for tools

Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
---
M docs/release_notes.adoc
1 file changed, 4 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6dc80b356d4cec6d0065a515dce5fdd94581346a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [doc] updated release notes for Kudu C++ client
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5006/2/docs/release_notes.adoc
File docs/release_notes.adoc:

Line 62: 
Nit: the three bullet points refer to a client as either a " client 
library", a " client API", or a " client". We should 
consolidate behind two of those if not just one. I'd personally vote for the 
last one as it's the shortest, but I'll defer to your judgment.


PS2, Line 74: or invocation of the callback passed into 
KuduSession::FlushAsync()
Nit: "or, if using asynchronous flushing, since last invocation of the callback 
passed into KuduSession::FlushAsync()).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Add IN LIST release notes

[Dan Burkert (Code Review)]

Hello Jean-Daniel Cryans,

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

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

to review the following change.

Change subject: Add IN LIST release notes
..

Add IN LIST release notes

Change-Id: I61a6a1871098a3252e09556ab510ca9693d14d74
---
M docs/release_notes.adoc
1 file changed, 4 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I61a6a1871098a3252e09556ab510ca9693d14d74
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR] WIP: workarounds for el6/krb5 1.10

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: WIP: workarounds for el6/krb5 1.10
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4990/4/src/kudu/security/test/krb5_realm_override.cc
File src/kudu/security/test/krb5_realm_override.cc:

Line 74:   CHECK(g_orig_krb5_get_host_realm)
Is there any way get_host_realm would be found, but not get_default_realm or 
free_default_realm?  May want to check all three just to be safe.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [doc] updated release notes for Kudu C++ client
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5006/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 70: {cpp} client API
> Nit: can you format this like the first point? Something like "The {cpp} cl
Done


PS1, Line 71: as deprecated
> Instead of "marked as deprecated", just say "deprecated".
Done


PS1, Line 72: Besides, it's possible to count number of buffered
:   operations by a different means.
> Let's be more direct. "Instead, count the number of buffered operations by 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [doc] updated release notes for Kudu C++ client
..

[doc] updated release notes for Kudu C++ client

Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
---
M docs/release_notes.adoc
1 file changed, 8 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [c++ client] implemented session operations stats

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [c++ client] implemented session operations stats
..


Patch Set 3:

(7 comments)

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

PS3, Line 9:  
missing a word? "is"?


PS3, Line 10: in the context
s/in the context/during the lifetime


PS3, Line 10: The stats are not reset
: during the lifetime of a session.
with the fix to the previous sentence I don't think you need this one (its 
implicit)


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

PS3, Line 2686: EXPECT_EQ(1, session->total_write_ops());
  :   EXPECT_EQ(0, session->failed_write_ops());
Is it really interesting to be checking the stats in all these places? this 
will make changing these tests harder. Are we really measuring something useful?


http://gerrit.cloudera.org:8080/#/c/4974/3/src/kudu/client/client.h
File src/kudu/client/client.h:

PS3, Line 1505:  
.. of _valid_/_successful_ invocations?


PS3, Line 1518: and
nit: s/and/or


PS3, Line 1526: failed_write_ops(
on a more general note: does it make sense to measure the failed calls to 
Apply()? At first glance I would think this would be interesting to measure 
Kudu client/server errors not API misusage, or did I miss something?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb4191105158f49d811763fdc8e82831b4e2e0be
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] macOS: fix compile error introduced by 141de3377e6bdd

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: macOS: fix compile error introduced by 141de3377e6bdd
..


Patch Set 2: Code-Review+2

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

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

[kudu-CR] macOS: fix Kerberos tests where possible

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: macOS: fix Kerberos tests where possible
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5005/1/src/kudu/rpc/sasl_rpc-test.cc
File src/kudu/rpc/sasl_rpc-test.cc:

Line 262:   CHECK_ERR(setenv("KRB5_KTNAME", kt_path.c_str(), 1 /*replace*/));
> Won't setenv() leak into the run of other tests in the same file? If so, wo
KuduTest::SetUp() overrides the krb5 environment back to invalid paths so that 
each test case is self-contained (sort of like how flagsaver preserves gflag 
values)


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

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

[kudu-CR] WIP: workarounds for el6/krb5 1.10

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: WIP: workarounds for el6/krb5 1.10
..


Patch Set 3:

Awesome, this is looking a lot better.  CI linker issues look legit.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] KUDU-1563. Add support for INSERT IGNORE

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1563. Add support for INSERT IGNORE
..


Patch Set 11:

(3 comments)

only nits and one possible patch split.
Could you address the tidy bot nits?

Thanks for being persistent Brock, almost there :)

http://gerrit.cloudera.org:8080/#/c/4491/11/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:

Line 280: for (uint32_t feature : op->write_op->required_server_features()) 
{
hummm this smells like it should go on its own patch along with the other 
feature flag stuff and with a small directed test. Would that be hard?


http://gerrit.cloudera.org:8080/#/c/4491/11/src/kudu/tablet/tablet_random_access-test.cc
File src/kudu/tablet/tablet_random_access-test.cc:

PS11, Line 113: int r = rand() % 3;
move 'r' and its assignment to before the outer if


PS11, Line 114: if (r == 1) {
use switch/case


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bfc35e9d27bd5e2d3375b68e6e4716ed671f36c
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Brock Noland 
Gerrit-Reviewer: Brock Noland 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP: workarounds for el6/krb5 1.10

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP: workarounds for el6/krb5 1.10
..

WIP: workarounds for el6/krb5 1.10

Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
---
M src/kudu/integration-tests/external_mini_cluster-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/security/CMakeLists.txt
A src/kudu/security/init.cc
A src/kudu/security/init.h
A src/kudu/security/test/krb5_realm_override.cc
M src/kudu/server/CMakeLists.txt
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_main.cc
10 files changed, 298 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] macOS: fix compile error introduced by 141de3377e6bdd

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: macOS: fix compile error introduced by 141de3377e6bdd
..


Patch Set 1: Code-Review+2

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

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

[kudu-CR] macOS: fix compile error introduced by 141de3377e6bdd

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: macOS: fix compile error introduced by 141de3377e6bdd
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5009/1/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
File src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc:

Line 187: int64_t ExternalMiniClusterFsInspector::GetTabletSuperBlockMTimeOrDie(
> Hmm. Not your fault obviously, but I have some requests for other (small) c
but this is a time-since-epoch whereas MonoTime is 
time-since-arbitrary-reference-point. It's just that timespec is somewhat 
overloaded in posix APIs.


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

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

[kudu-CR] macOS: fix compile error introduced by 141de3377e6bdd

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: macOS: fix compile error introduced by 141de3377e6bdd
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5009/1/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
File src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc:

Line 187: int64_t ExternalMiniClusterFsInspector::GetTabletSuperBlockMTimeOrDie(
Hmm. Not your fault obviously, but I have some requests for other (small) 
changes here:

1) Could you drop the std:: prefix from std::string on L188?
2) Could you add a static MonoTime::FromTimeSpec() method that mirrors 
MonoTime::ToTimeSpec(), change this method to use it, and return a MonoTime 
here instead?


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

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

[kudu-CR] KUDU-237 (part 2) - Add support for REINSERT in delta files

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-237 (part 2) - Add support for REINSERT in delta files
..

KUDU-237 (part 2) - Add support for REINSERT in delta files

This patch goes the final mile in adding support for REINSERTs in
delta files. It does various things to achieve this:

- Transforms REDO DELETE/REINSERT mutation pairs into UNDO
REINSERT/DELETE mutation pairs, leaving at the most a REDO
delete.

- Merges ghost rows on compaction: When duplicated rows are
found a new algorithm finds out which one is the most recent
and adds the other one as a 'previous_version'. This can happen
for an arbitrary number of ghost rows. Of interest here is that
this process requires a copy. Since the oldest ghost row is
discarded from the compaction input we must make sure that its
data survives until the most recent is compacted. This is done
by copying the oldest to the youngest's arena.

- Adds new test to tablet-test and changes a test in
compaction-test proving that this works.

Follow up patches will add new itests that test this functionality
more broadly.

Change-Id: Ie1173b2bea721b376f2b6049be20f57307582c47
---
M src/kudu/common/row_changelist.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/tablet-test.cc
7 files changed, 376 insertions(+), 130 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie1173b2bea721b376f2b6049be20f57307582c47
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add snapshot scans to fuzz-itest

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add snapshot scans to fuzz-itest
..

Add snapshot scans to fuzz-itest

This adds a new operation to fuzz-itest: snapshot scans
at timestamp.

When generating random operations scans at a timestamp are
now in the mix. The generator records how timestamps will
progress with writes and is careful to make sure that scans
happen in the past. After the test case is generated, but
before it is run, the timestamps for the scans are deduplicated
and sorted to that we save state at those (and only those)
timestamps while running.

This would fail very often before the REINSERT patches but
passes with it.

WIPish: This is just missing some dist-test runs but should
otherwise be ready for review.

Change-Id: I4d15129e83c5c9afa9b643e674c8a23e18a2fa0e
---
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/tablet/key_value_test_schema.h
M src/kudu/tserver/tablet_service.cc
3 files changed, 197 insertions(+), 32 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d15129e83c5c9afa9b643e674c8a23e18a2fa0e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] macOS: fix compile error introduced by 141de3377e6bdd

[Dan Burkert (Code Review)]

Hello Adar Dembo, Todd Lipcon,

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

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

to review the following change.

Change subject: macOS: fix compile error introduced by 141de3377e6bdd
..

macOS: fix compile error introduced by 141de3377e6bdd

Change-Id: Id2a9556810feeb3128b97d60afb18a2547e0af5c
---
M src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
1 file changed, 4 insertions(+), 0 deletions(-)


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

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

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [doc] updated release notes for Kudu C++ client
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5006/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 70: {cpp} client API
Nit: can you format this like the first point? Something like "The {cpp} 
client's KuduSession::CountBufferedOperations() method...".


PS1, Line 71: as deprecated
Instead of "marked as deprecated", just say "deprecated".


PS1, Line 72: Besides, it's possible to count number of buffered
:   operations by a different means.
Let's be more direct. "Instead, count the number of buffered operations by ..."


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [tools] Manual recovery tools (part 1)

[Dinesh Bhat (Code Review)]

Hello Mike Percy, Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [tools] Manual recovery tools (part 1)
..

[tools] Manual recovery tools (part 1)

This change introduces two recovery tools:
1) 'kudu remote_replica copy' copies the given tablet onto
   a destination server irrespective of the state of the
   destination replica (--force option provided).
   This is a port from https://gerrit.cloudera.org/#/c/3582/
2) 'kudu local_replica delete' (KUDU-1618) deletes a local replica
   of a tablet when the tablet server can not come up due to a
   bad replica of a tablet. This tool should be used with caution,
   hence added a --clean_unsafe flag to warn the user with
   consequences. As of this change, tool supports
   deleting the tablet permanently if used with --clean_unsafe flag,
   and fails with 'unsupported action error' without the flag.
   A future patch will add a default action of tombstoning
   the tablet instead of permanently deleting it.

Also added few tests exercising typical usage scenario for these tools.
Added trace to stderr under existing RunAction routines to throw more
info in the log if an action fails.

Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
11 files changed, 456 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/4834/17
-- 
To view, visit http://gerrit.cloudera.org:8080/4834
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [tools] Manual recovery tools (part 1)

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 15:

(3 comments)

Also updated COMMIT_MSG to reflect the new flag added.

http://gerrit.cloudera.org:8080/#/c/4834/13/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:

Line 80: DEFINE_bool(clean_unsafe, false,
> Hmm, --force_unsafe, I guess I would prefer either --force or --clean_unsaf
Done


Line 81: "Delete the local replica completely, not leaving a 
tombstone. "
> How about: Delete the local replica completely, not leaving a tombstone. Th
Done


Line 270: return Status::NotSupported(Substitute("Deletion of local replica 
$0 is "
> s/not supported/currently not supported/
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [tools] Manual recovery tools (part 1)

[Dinesh Bhat (Code Review)]

Hello Mike Percy, Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [tools] Manual recovery tools (part 1)
..

[tools] Manual recovery tools (part 1)

This change introduces two recovery tools:
1) 'kudu remote_replica copy' copies the given tablet onto
   a destination server irrespective of the state of the
   destination replica (--force option provided).
   This is a port from https://gerrit.cloudera.org/#/c/3582/
2) 'kudu local_replica delete' (KUDU-1618) deletes a local replica
   of a tablet when the tablet server can not come up due to a
   bad replica of a tablet. This tool should be used with caution,
   hence added a --clean_unsafe flag to warn the user with
   consequences. As of this change, tool supports
   deleting the tablet permanently if used with --clean_unsafe flag,
   and fails with 'unsupported action error' without the flag.
   A future patch will add a default action of tombstoning
   the tablet instead of permanently deleting it.

Also added few tests exercising typical usage scenario for these tools.
Added trace to stderr under existing RunAction routines to throw more
info in the log if an action fails.

Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
11 files changed, 456 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/34/4834/16
-- 
To view, visit http://gerrit.cloudera.org:8080/4834
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: workarounds for el6/krb5 1.10

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP: workarounds for el6/krb5 1.10
..

WIP: workarounds for el6/krb5 1.10

Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
---
M src/kudu/integration-tests/external_mini_cluster-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/security/CMakeLists.txt
A src/kudu/security/init.cc
A src/kudu/security/init.h
A src/kudu/security/test/krb5_realm_override.cc
M src/kudu/server/CMakeLists.txt
M src/kudu/server/server_base.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_main.cc
10 files changed, 288 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I708334cbbee35d2629a38a369e63c1dc309ed91b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-1634. TS and MS should delete tmp metadata files on startup

[Maxim Smyatkin (Code Review)]

Maxim Smyatkin has uploaded a new change for review.

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

Change subject: KUDU-1634. TS and MS should delete tmp metadata files on startup
..

KUDU-1634. TS and MS should delete tmp metadata files on startup

Initially it was suggested that TS should delete tmp files from consensus-meta
directory, but these leftovers can appear anywhere where atomic write with
WritePBContainerToPath() is being used. For now, it is consensus metadata,
black manager's data directory, instance metadata, etc. Also, it happens to
Master server as well.
Hence, I've decided to deal with it on FsManager level and scan all root paths
for tmp files. It may slow startup a little bit, but I don't know any way
around it because:
a) it can't be done on background as it can remove the real tmp files
this way;
b) it would be nice to have a separate tmp directory, but it makes rename not
atomic this way (because we can - and usually will - have root/wal dirs on
different drives).

Change-Id: I299e7b22c3f920430ff5d581cf7a507a83eb0101
---
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
3 files changed, 95 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I299e7b22c3f920430ff5d581cf7a507a83eb0101
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Maxim Smyatkin 

[kudu-CR] [tools] Manual recovery tools (part 1)

[Dinesh Bhat (Code Review)]

Hello Mike Percy, Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [tools] Manual recovery tools (part 1)
..

[tools] Manual recovery tools (part 1)

This change introduces two recovery tools:
1) 'kudu remote_replica copy' copies the given tablet onto
   a destination server irrespective of the state of the
   destination replica (--force option provided).
   This is a port from https://gerrit.cloudera.org/#/c/3582/
2) 'kudu local_replica delete' (KUDU-1618) deletes a local replica
   of a tablet when the tablet server can not come up due to a
   bad replica of a tablet. This tool should be used with caution,
   hence added a --clean_unsafe flag to warn the user with
   consequences. As of this change, tool supports
   deleting the tablet permanently if used with --clean_unsafe flag,
   and fails with 'unsupported action error' without the flag.
   A future patch will add a default action of tombstoning
   the tablet instead of permanently deleting it.

Also added few tests exercising typical usage scenario for these tools.
Added trace to stderr under existing RunAction routines to throw more
info in the log if an action fails.

Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
11 files changed, 456 insertions(+), 65 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [doc] updated release notes for Kudu C++ client

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [doc] updated release notes for Kudu C++ client
..

[doc] updated release notes for Kudu C++ client

Change-Id: If4667dc7bb90fdf910c06a9107ce41f1ac3d7037
---
M docs/release_notes.adoc
1 file changed, 7 insertions(+), 2 deletions(-)


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

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

[kudu-CR] rpc: fix too-long memcmp for HTTP methods

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: rpc: fix too-long memcmp for HTTP methods
..


rpc: fix too-long memcmp for HTTP methods

Previously we were using arraysize(kHTTPHeader) where kHTTPHeader is a
constant "HTTP". This array would then include the terminating null
byte, which is length 5 rather than the expected length 4.
This switches to using strlen instead of arraysize.

This fixes the following ASAN error seen occasionally in tests:

==5170==ERROR: AddressSanitizer: use-after-poison on address 0x7f571f7b886c at 
pc 0x0048b40c bp 0x7f571f7b8370 sp 0x7f571f7b7b20
READ of size 5 at 0x7f571f7b886c thread T85 (negotiator [wor)
#0 0x48b40b in memcmp 
/data1/jenkins-workspace/kudu-workspace/thirdparty/src/llvm-3.9.0.src/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:628
#1 0x7f574d3c126d in kudu::rpc::ReceiveFramedMessageBlocking(kudu::Socket*, 
kudu::faststring*, google::protobuf::MessageLite*, kudu::Slice*, kudu::MonoTime 
const&) 
/data1/jenkins-workspace/kudu-workspace/src/kudu/rpc/blocking_ops.cc:103:9

Change-Id: Ic3f7be076bf0e1459d583876f65036e31034adcb
Reviewed-on: http://gerrit.cloudera.org:8080/4992
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/rpc/blocking_ops.cc
1 file changed, 2 insertions(+), 1 deletion(-)

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



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

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

[kudu-CR] [tools] Manual recovery tools (part 1)

[Dinesh Bhat (Code Review)]

Hello Mike Percy, Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [tools] Manual recovery tools (part 1)
..

[tools] Manual recovery tools (part 1)

This change introduces two recovery tools:
1) 'kudu remote_replica copy' copies the given tablet onto
   a destination server irrespective of the state of the
   destination replica (--force option provided).
   This is a port from https://gerrit.cloudera.org/#/c/3582/
2) 'kudu local_replica delete' (KUDU-1618) deletes a local replica
   of a tablet when the tablet server can not come up due to a
   bad replica of a tablet.

Also added few tests exercising typical usage scenario for these tools.
Added trace to stderr under existing RunAction routines to throw more
info in the log if an action fails.

Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
11 files changed, 456 insertions(+), 65 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [c++ client] implemented session operations stats

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [c++ client] implemented session operations stats
..


Patch Set 3:

As it turned out, this change is not very useful if not adding additional 
fine-grained stats on types of errors.  Given the fact it modifies the public 
client API, it does not make sense to continue with it in the absence of demand 
from the consumer side.

Will need to return back with more systematic approach for fine-grained stats 
for errors.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb4191105158f49d811763fdc8e82831b4e2e0be
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Release notes for the Java client in 1.1

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Release notes for the Java client in 1.1
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/5002/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 50: troubleshooting
troubleshoot


PS1, Line 54:  will not
"are not" (twice in this sentence)


PS1, Line 59: currently
:   available
Replace with "existing"


PS1, Line 86: are not using the default
:   timeouts anymore to wait for the requests to complete. 
This part may be too much detail. Maybe it's enough to just say that before the 
sync API could throw two kinds of exceptions, now just one?


Line 91: * The Java client's handling of errors in `KuduSession` was modified 
so that subclasses of
Shouldn't this specifically refer to Flush() or Apply()? Not clear where these 
exceptions were being thrown from, or under what circumstances.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf04d1db4769e19e6421628bbe980cb4c058c62c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java client] Wrong javadoc in KuduClient

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: [java client] Wrong javadoc in KuduClient
..


[java client] Wrong javadoc in KuduClient

Timeouts on join were removed in the sync API but KuduClient's javadoc wasn't 
adjusted.

Change-Id: Iddf2e33b82002535eb1f8baefadc1405a9c2
Reviewed-on: http://gerrit.cloudera.org:8080/5001
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
1 file changed, 2 insertions(+), 3 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iddf2e33b82002535eb1f8baefadc1405a9c2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Wrong javadoc in KuduClient

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java client] Wrong javadoc in KuduClient
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iddf2e33b82002535eb1f8baefadc1405a9c2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Add snapshot scans to fuzz-itest

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add snapshot scans to fuzz-itest
..

Add snapshot scans to fuzz-itest

This adds a new operation to fuzz-itest: snapshot scans
at timestamp.

When generating random operations scans at a timestamp are
now in the mix. The generator records how timestamps will
progress with writes and is careful to make sure that scans
happen in the past. After the test case is generated, but
before it is run, the timestamps for the scans are deduplicated
and sorted to that we save state at those (and only those)
timestamps while running.

This would fail very often before the REINSERT patches but
passes with it.

WIPish: This is just missing some dist-test runs but should
otherwise be ready for review.

Change-Id: I4d15129e83c5c9afa9b643e674c8a23e18a2fa0e
---
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/tablet/key_value_test_schema.h
M src/kudu/tserver/tablet_service.cc
3 files changed, 199 insertions(+), 31 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d15129e83c5c9afa9b643e674c8a23e18a2fa0e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add Reinserts to tablet history gc-itest

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add Reinserts to tablet_history_gc-itest
..

Add Reinserts to tablet_history_gc-itest

This adds Reinserts as a new operation to tablet_history_gc-itest.
Reinserts happen on previously deleted rows and, like deletes,
have a change to happen mid-compaction.

This test used to fail frequently before the reinsert patches
are in and now passes.

WIPish Will likely get a few dist-test runs for this, though
this was already flaky so we'll see how troublesome that is..

Change-Id: I6b774b14d79bd7c0c0bf62b585bd70d6d975feaa
---
M src/kudu/integration-tests/tablet_history_gc-itest.cc
1 file changed, 90 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6b774b14d79bd7c0c0bf62b585bd70d6d975feaa
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [tools] Manual recovery tools (part 1)

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: [tools] Manual recovery tools (part 1)
..


Patch Set 13:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4834/13/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:

Line 80: DEFINE_bool(force_unsafe, false,
Hmm, --force_unsafe, I guess I would prefer either --force or --clean_unsafe 
since "force" implies unsafe in my mind but "clean" is more descriptive.


Line 81: "Delete the local replica permanently which may "
How about: Delete the local replica completely, not leaving a tombstone. This 
is not guaranteed to be safe because it also removes the consensus metadata 
(including Raft voting record) for the specified tablet, which violates the 
Raft vote durability requirements.


Line 270: "not supported without --force_unsafe flag", tablet_id));
s/not supported/currently not supported/


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I113a25e9b6c14f7c3814140917b61e35030b58d0
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP: Don't output unobservable rows from the MemRowset

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP: Don't output unobservable rows from the MemRowset
..

WIP: Don't output unobservable rows from the MemRowset

In some rare cases we might have a row that is inserted and deleted
in the same operation, meaning the insert and delete have the same
timestamp. In this rare case we might run into trouble later on with
compactions as there is a sequence of operations that that produce
two ghost rows which are uncomparable in terms of recency.

The sequence is as follows:

Insert Row at 1
Delete Row at 3
Flush - Ghost 1 now lives in RS1
Insert Row at 3
Delete Row at 3
Flush - Ghost 2 now lives in RS2
Major delta compact RS1 - GC all before 3
Major delta compact RS2 - GC all before 3

If RS1 and RS2 now get compacted together there is no way to
distinguish Ghost 1 from Ghost 2 and to build a correct history
for the row.

WIP as this is missing a small directed test.

Change-Id: Ieab52a1aa68494218f91f3acd31ef8ddf352bd57
---
M src/kudu/tablet/compaction.cc
1 file changed, 46 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieab52a1aa68494218f91f3acd31ef8ddf352bd57
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon