[kudu-CR] KUDU-1897: disable Kerberos replay cache

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: KUDU-1897: disable Kerberos replay cache
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6254/2/src/kudu/rpc/rpc-negotiation-bench.cc
File src/kudu/rpc/rpc-negotiation-bench.cc:

PS2, Line 130: StartTestServerWithGeneratedCode(_addr_)
Is it necessary to enable TLS support if running with TLS-enabled configuration?


PS2, Line 214: Acquire_Load(_run_)
Consider using atomic for should_run_ since atomic is introduced for 
negotiation_count.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbce55a0b12682fdf69e7b2c361c6336495db64d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
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] [catalog manager] categorization of rw operation failures

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [catalog_manager] categorization of rw operation failures
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6170/9/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

PS9, Line 546: PrepareForMasterLeadershipTask
> You mean PrepareForLeadershipTask().
Yes, exactly -- thanks!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] categorization of rw operation failures

[Alexey Serbin (Code Review)]

Hello Adar Dembo,

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

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

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

Change subject: [catalog_manager] categorization of rw operation failures
..

[catalog_manager] categorization of rw operation failures

This changelist introduces the categorization of the system catalog's
read and write operation failures which happen on leader post-election
callback.  There are two categories of errors: fatal and non-fatal.

If an operation against the system catalog fails in between terms of
the catalog leadership, the error is considered non-fatal.  In case of
a non-fatal error the leader post-election task bails out: the catalog
is no longer the leader at the original term and the task should be
executed by the new leader upon ElectedAsLeaderCb.

If an operation against the system catalog fails at the same term of the
catalog leadership, the error is considered fatal and that causes the
master process to crash.  This is to avoid possible inconsistency when
working with the tables/tablets metadata, the IPKI certificate authority
information and TSKs (Token Signing Keys).

Any failure of a read or write operation against the system catalog
happened during the catalog's shutdown is ignored and the leader
post-election task bails out once detecting such failure.

The same policy applies to other (i.e. not specific to read and write
operations against the system catalog) errors which might happen while
working with the IPKI certificate authority information and TokenSigner.
The rationale is the same as for handling the system catalog operation
failures: in case of an error, the leader has no consistent information
to work with, meanwhile a non-leader does not use the information
affected by the failure at all and can safely ignore the error.

Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
6 files changed, 235 insertions(+), 135 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [catalog manager] categorization of rw operation failures

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [catalog_manager] categorization of rw operation failures
..


Patch Set 9: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6170/9/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

PS9, Line 546: PrepareForMasterLeadershipTask
You mean PrepareForLeadershipTask().


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Reserve 1% of disk space by default

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Reserve 1% of disk space by default
..


Patch Set 3: Code-Review+2

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

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

[kudu-CR] Ignore SIGPIPE earlier in startup process

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Ignore SIGPIPE earlier in startup process
..


Patch Set 1:

(3 comments)

> > What's the plan for the client?
 > 
 > I thought we could deal with the client in a separate patch. The
 > idea I had there was to add an option to KuduClientBuilder called
 > ignore_sigpipe(bool) that defaults to true. If clients want to
 > manage their own signal handling themselves, at a process- or
 > thread- level, then they can set it to false when constructing the
 > client. If it's set to true, it calls something like
 > EnsureSigPipeIgnored().

I don't think we even have to go that far; documenting that it's the 
application's responsibility to ignore SIGPIPE before setting up a TLS-enabled 
KuduClient should suffice. Unlike ignore_sigpipe(bool), this can be removed in 
the future once we do the BIO wrapper thing (mentioned in a different comment).

We should also find out whether Impala is vulnerable to this. Like us, 
squeasel's sq_start() will configure SIGPIPE to SIG_IGN, but also like us, it's 
possible that they may have called send() on TLS sockets by then.

http://gerrit.cloudera.org:8080/#/c/6262/1/src/kudu/util/logging.cc
File src/kudu/util/logging.cc:

Line 269:   // Ignore SIGPIPE early in the startup process so that threads 
writing to TLS
Can you file a JIRA about this? Last night Todd mentioned that we can implement 
a BIO wrapper for openssl that'll let us explicitly set MSG_NOSIGNAL in send() 
calls. Once that happens, I'd argue we should remove this code.


http://gerrit.cloudera.org:8080/#/c/6262/1/src/kudu/util/logging.h
File src/kudu/util/logging.h:

PS1, Line 262: ignore
Nit: SIG_IGN, to be more precise.


http://gerrit.cloudera.org:8080/#/c/6262/1/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:

Line 76: typedef sighandler_t SignalHandlerCallback;
Is this still needed?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I040bd38ff31451ed9e25e7cf2127c869cf08a628
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Ignore SIGPIPE earlier in startup process

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Ignore SIGPIPE earlier in startup process
..


Patch Set 1:

(1 comment)

> What's the plan for the client?

I thought we could deal with the client in a separate patch. The idea I had 
there was to add an option to KuduClientBuilder called ignore_sigpipe(bool) 
that defaults to true. If clients want to manage their own signal handling 
themselves, at a process- or thread- level, then they can set it to false when 
constructing the client. If it's set to true, it calls something like 
EnsureSigPipeIgnored().

http://gerrit.cloudera.org:8080/#/c/6262/1/src/kudu/util/signal.cc
File src/kudu/util/signal.cc:

PS1, Line 38:   static GoogleOnceType once = GOOGLE_ONCE_INIT;
:   GoogleOnceInit(, );
> I think you just moved this code, but actually curious why we bother using 
Yeah I did just move this code... I assumed it's to avoid a syscall when we 
think we don't need it. I'm a little ambivalent on it and do think avoiding a 
static GoogleOnce is good for testability, so I'm not opposed to removing this.

Actually, I would like to remove the call to this from Subprocess::Start() but 
I'm a little worried about opening us up to additional test flakiness because 
of that. On the other hand, the call there could be masking other issues.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I040bd38ff31451ed9e25e7cf2127c869cf08a628
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race

[Mike Percy (Code Review)]

Mike Percy has abandoned this change.

Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
..


Patch Set 1:

I thought fork included all threads. I guess the difference is that the child 
gets all the memory but only one of the threads. In that case, we don't need 
this patch after all. TFTR, I'll abandon.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Make ExternalDaemon::StartProcess() handle fault injection

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Make ExternalDaemon::StartProcess() handle fault injection
..


Patch Set 6:

Is this patch necessary if we pull in https://gerrit.cloudera.org/6262 ?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6046e34a321de3e324e20e3d63249e4073712447
Gerrit-PatchSet: 6
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] env: Add support for getting FS capacity

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: env: Add support for getting FS capacity
..


Patch Set 1: Verified+1

Overriding jenkins flakiness

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id43275876d3352f5cf943e24ed4513b9f2c131aa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] [security] added TokenSigner::IsCurrentKeyValid() method

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [security] added TokenSigner::IsCurrentKeyValid() method
..


Patch Set 3: Verified+1

Flake: org.apache.kudu.client.ITClient.test

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f85c166c431951d2d676e6936e4bc3b965318c9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [catalog manager] categorization of rw operation failures

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [catalog_manager] categorization of rw operation failures
..


Patch Set 9: Verified+1

Flake: org.apache.kudu.spark.kudu.DefaultSourceTest.Test SparkSQL 
StringStartsWith filters

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Ignore SIGPIPE earlier in startup process

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Ignore SIGPIPE earlier in startup process
..


Patch Set 1:

(1 comment)

What's the plan for the client?

http://gerrit.cloudera.org:8080/#/c/6262/1/src/kudu/util/signal.cc
File src/kudu/util/signal.cc:

PS1, Line 38:   static GoogleOnceType once = GOOGLE_ONCE_INIT;
:   GoogleOnceInit(, );
I think you just moved this code, but actually curious why we bother using a 
ONCE here. it's not harmful to set twice


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I040bd38ff31451ed9e25e7cf2127c869cf08a628
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
..


Patch Set 1:

Not sure I follow this. Between fork and exec there is only one thread running 
(fork only forks the thread that calls it).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [security] added TokenSigner::IsCurrentKeyValid() method

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [security] added TokenSigner::IsCurrentKeyValid() method
..

[security] added TokenSigner::IsCurrentKeyValid() method

Introduced TokenSigner::IsCurrentKeyValid() method to check whether
TokenSigner's current key is valid.  Added a unit test as well.

Change-Id: I2f85c166c431951d2d676e6936e4bc3b965318c9
---
M src/kudu/security/token-test.cc
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signer.h
3 files changed, 39 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f85c166c431951d2d676e6936e4bc3b965318c9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
..


Patch Set 1: Verified+1

Overriding unrelated failure due to KUDU-1736 (I attached the failure log to 
that JIRA)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] [catalog manager] categorization of rw operation failures

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [catalog_manager] categorization of rw operation failures
..


Patch Set 8:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/6170/8//COMMIT_MSG
Commit Message:

PS8, Line 10: d/write operation failures happened while executing the leader
: post-election callback
> sentence fragment
Done


PS8, Line 14: /
> Make it explicit whether this means 'and' or 'or'.
Done


http://gerrit.cloudera.org:8080/#/c/6170/8/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

Line 463:   const Status& s = 
catalog_manager_->ProcessPendingAssignments(to_process);
> Would you mind making this an owned Status here and below?  I find the by-v
Done


PS8, Line 763: found
> is found
Done


PS8, Line 770:  
> the master
Done


PS8, Line 774: already
 : // contain CA certificate information written by the new 
leader master
> contain the CA certificate information when this master is re-elected next.
Done


Line 790:   return s;
> I'm guessing this is an unnecessary copy because the status is kept as a re
That's a good point.


PS8, Line 911: const Consensus& consensus
> this could just be captured.
If you don't mind, I would prefer to leave it as it is for better modularity.


Line 914:   const int64_t term_pre =
> Why not use 'term'?
Because there is pairing 'term_post'.  I updated this code so this is no longer 
an issue.


Line 938: LOG(WARNING) << op_description << " failed; "
> This is logged at INFO level on line 899, why WARNING here?
It's in the context of the 'if (!s.ok())' path -- some operation has failed.  
But since you noticed this, I think it's better to change it to INFO as well 
because the failed operation means we are not using that anyway and some other 
master server picked this up.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] categorization of rw operation failures

[Alexey Serbin (Code Review)]

Hello Adar Dembo,

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

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

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

Change subject: [catalog_manager] categorization of rw operation failures
..

[catalog_manager] categorization of rw operation failures

This changelist introduces the categorization of the system catalog's
read and write operation failures happened on leader post-election
callback.  There are two categories of errors: fatal and non-fatal.

If system catalog operation fails in between terms of the system catalog
leadership, the error is considered non-fatal.  In case of a non-fatal
error the leader post-election task bails out: the catalog is no longer
the leader at the original term and the task should be executed by the
new leader upon ElectedAsLeaderCb.

If system catalog operation failure happened at the same term of the
system catalog leadership, the error is considered fatal and that causes
the master process to crash.  This is to avoid possible inconsistency
when working with the tables/tablets metadata, the IPKI certificate
authority information and TSKs (Token Signing Keys).

All read and write system catalog operation failures happened during
the catalog's shutdown are ignored and the leader post-election task
bails out when detecting that.

The same policy applies to other (i.e. not specific to the system
catalog) errors which might happen while working with the IPKI
certificate authority information and TokenSigner.  The reason is
the same as with the system catalog operation failures: in case of an
error, the leader has no consistent information to work with, meanwhile
a non-leader does not use that information at all and can safely ignore
the error.

Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
6 files changed, 235 insertions(+), 135 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [security] introduced TokenSigner::HasValidKey() method

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [security] introduced TokenSigner::HasValidKey() method
..

[security] introduced TokenSigner::HasValidKey() method

Introduced the TokenSigner::HasValidKey() method to check whether
TokenSigner has valid key to sign tokens.  Added a unit test as well.

Change-Id: I2f85c166c431951d2d676e6936e4bc3b965318c9
---
M src/kudu/security/token-test.cc
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signer.h
3 files changed, 39 insertions(+), 1 deletion(-)


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

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

[kudu-CR] Ignore SIGPIPE earlier in startup process

[Mike Percy (Code Review)]

Hello David Ribeiro Alves,

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

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

to review the following change.

Change subject: Ignore SIGPIPE earlier in startup process
..

Ignore SIGPIPE earlier in startup process

This change resolves a race during startup where we are not protected
from SIGPIPE from the time we start the process until the time we start
the squeasel web server, which sets the disposition of SIGPIPE to
SIG_IGN.

This also factors some of the signal-handling helper functions into a
new set of util files, signal.{h,cc}.

Change-Id: I040bd38ff31451ed9e25e7cf2127c869cf08a628
---
M src/kudu/util/CMakeLists.txt
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
A src/kudu/util/signal.cc
A src/kudu/util/signal.h
M src/kudu/util/subprocess.cc
6 files changed, 97 insertions(+), 26 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I040bd38ff31451ed9e25e7cf2127c869cf08a628
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 

[kudu-CR] KUDU-1905 - Allow reinserts on pk only tables

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1905 - Allow reinserts on pk only tables
..

KUDU-1905 - Allow reinserts on pk only tables

Doing a reinsert to a table that has only primary key columns
results in an empty change list. We're currently crashing whenever
we see a empty changelist that is not a delete.

The fix is just to allow empty changelists for reinserts.
This also adds a new flavor of fuzz tests to fuzz-itest.cc
that only have pk-only operations, as well as a directed
regression test that would trigger the problem deterministically.

Ran fuzz-itest in dist-tests with the new tests and the following
command:
KUDU_ALLOW_SLOW_TESTS=1 build-support/dist_test.py --collect-tmpdir \
loop -n 1000 build/debug/bin/fuzz-itest --gtest_repeat=10 \
--gtest_break_on_failure

Tests passed 1000/1000. Results:
http://dist-test.cloudera.org//job?job_id=david.alves.1488580839.22665

Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
---
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_changelist.cc
M src/kudu/integration-tests/fuzz-itest.cc
3 files changed, 118 insertions(+), 31 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1905 - Allow reinserts on pk only tables

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1905 - Allow reinserts on pk only tables
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6258/2/src/kudu/integration-tests/fuzz-itest.cc
File src/kudu/integration-tests/fuzz-itest.cc:

PS2, Line 446:   case TEST_INSERT:
 : if (exists[row_key]) continue;
 : ops->push_back({TEST_INSERT, row_key});
 : exists[row_key] = true;
 : ops_pending = true;
 : data_in_mrs = true;
 : break;
 :   case TEST_INSERT_PK_ONLY:
 : if (exists[row_key]) continue;
 : ops->push_back({TEST_INSERT_PK_ONLY, row_key});
 : exists[row_key] = true;
 : ops_pending = true;
 : data_in_mrs = true;
 :  
> you can combine these two cases by doing ops->push_back({r, row_key}) right
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1905 - Allow reinserts on pk only tables

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1905 - Allow reinserts on pk only tables
..


Patch Set 3:

(5 comments)

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

PS1, Line 9: ha
> has
Done


http://gerrit.cloudera.org:8080/#/c/6258/1/src/kudu/common/row_changelist.cc
File src/kudu/common/row_changelist.cc:

Line 163:   // Note that REINSERTs might have empty changelists when 
reinserting a row on a tablet that
> should we just say is_update()?
Done


http://gerrit.cloudera.org:8080/#/c/6258/2/src/kudu/integration-tests/fuzz-itest.cc
File src/kudu/integration-tests/fuzz-itest.cc:

Line 237:   ExpectedKeyValueRow InsertOrUpsertRow(int key, int val,
> nit: missing word 'if'
Done


PS2, Line 242: =
> typo
Done


PS2, Line 406: 
 : string final_error;
 : if (!errors.empty()) {
 :   
> what about doing something like:
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1905 - Allow reinserts on pk only tables

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1905 - Allow reinserts on pk only tables
..

KUDU-1905 - Allow reinserts on pk only tables

Doing a reinsert to a table that has only primary key columns
results in an empty change list. We're currently crashing whenever
we see a empty changelist that is not a delete.

The fix is just to allow empty changelists for reinserts.
This also adds a new flavor of fuzz tests to fuzz-itest.cc
that only have pk-only operations, as well as a directed
regression test that would trigger the problem deterministically.

Ran fuzz-itest in dist-tests with the new tests and the following
command:
KUDU_ALLOW_SLOW_TESTS=1 build-support/dist_test.py --collect-tmpdir \
loop -n 1000 build/debug/bin/fuzz-itest --gtest_repeat=10 \
--gtest_break_on_failure

Tests passed 1000/1000. Results:
http://dist-test.cloudera.org//job?job_id=david.alves.1488580839.22665

Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
---
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_changelist.cc
M src/kudu/integration-tests/fuzz-itest.cc
3 files changed, 123 insertions(+), 30 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race

[Mike Percy (Code Review)]

Hello David Ribeiro Alves,

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

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

to review the following change.

Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
..

subprocess: Get rid of fork/exec SIGPIPE reset race

There is a potential race in the current fork/exec procedure where,
after fork(), we set the signal disposition for SIGPIPE to SIG_DFL, and
then exec(). The problem with this sequence is that the parent process
may have outstanding threads performing socket or pipe writes which
could get interleaved between setting the signal handler to the default
and the exec().

The workaround in this patch is to set the signal handler in the child
to a no-op handler function in between fork() and exec(). Upon exec(),
the handler disposition is reset to SIG_DFL atomically and
automatically, resulting in the child getting SIG_DFL for SIGPIPE
without racy behavior.

Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
---
M src/kudu/util/subprocess.cc
1 file changed, 10 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 

[kudu-CR] KUDU-1905 - Allow reinserts on pk only tables

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1905 - Allow reinserts on pk only tables
..


Patch Set 2:

(6 comments)

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

PS1, Line 9: as
has


http://gerrit.cloudera.org:8080/#/c/6258/1/src/kudu/common/row_changelist.cc
File src/kudu/common/row_changelist.cc:

Line 163:   // Note that REINSERTs might have empty changelists when 
reinserting a row on a tablet that
should we just say is_update()?


http://gerrit.cloudera.org:8080/#/c/6258/2/src/kudu/integration-tests/fuzz-itest.cc
File src/kudu/integration-tests/fuzz-itest.cc:

Line 237: // For "upsert PK only", we expect the row to keep its old 
value
nit: missing word 'if'


PS2, Line 242: c
typo


PS2, Line 406: enum TestOpSets {
 :   ALL,
 :   PK_ONLY
 : };
what about doing something like:

vector kAllOps = { TEST_INSERT, TEST_UPDATE, TEST_UPSERT, 
TEST_INSERT_PK_ONLY, ...}
vector kPkOnlyOps = { TEST_INSERT_PK_ONLY, ...}

or somesuch? then the PickOpAtRandom can just pick a random one based on 
op_types.size() instead of the kind of funny "retry until we get one which 
matches" loop


PS2, Line 446:   case TEST_INSERT:
 : if (exists[row_key]) continue;
 : ops->push_back({TEST_INSERT, row_key});
 : exists[row_key] = true;
 : ops_pending = true;
 : data_in_mrs = true;
 : break;
 :   case TEST_INSERT_PK_ONLY:
 : if (exists[row_key]) continue;
 : ops->push_back({TEST_INSERT_PK_ONLY, row_key});
 : exists[row_key] = true;
 : ops_pending = true;
 : data_in_mrs = true;
 :  
you can combine these two cases by doing ops->push_back({r, row_key}) right?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1897: disable Kerberos replay cache

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: KUDU-1897: disable Kerberos replay cache
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6254/2/src/kudu/rpc/rpc-negotiation-bench.cc
File src/kudu/rpc/rpc-negotiation-bench.cc:

PS2, Line 148:   friend class ClientThread;
 :   friend class ClientAsyncWorkload;
It seems these declaration are not needed.


PS2, Line 190:  
nit: off-by-one shift


PS2, Line 198:   authn_token = SignedTokenPB();
 :   security::TokenPB token;
 :   token.set_expire_unix_epoch_seconds(WallTime_Now() + 
validity / 2);
 :   token.mutable_authn()->set_username("client-token");
 :   
ASSERT_TRUE(token.SerializeToString(authn_token->mutable_token_data()));
 :   ASSERT_OK(token_signer.SignToken(_token.get()));
Consider replacing with:

SignedTokenPB authn_token;
ASSERT_OK(token_signer.GenerateAuthnToken("client-token", _token));


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbce55a0b12682fdf69e7b2c361c6336495db64d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
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] [security] introduced TokenSigner::HasValidKey() method

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [security] introduced TokenSigner::HasValidKey() method
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6246/1/src/kudu/security/token_signer.cc
File src/kudu/security/token_signer.cc:

Line 164:   return (tsk_deque_.front()->expire_time() > WallTime_Now());
> Agree: HasValidKey() suggests this method checks for the presence of a vali
SGTM


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

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

[kudu-CR] KUDU-1330: Add a tool to unsafely recover from loss of majority replicas

[Dinesh Bhat (Code Review)]

Hello David Ribeiro Alves, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-1330: Add a tool to unsafely recover from loss of majority 
replicas
..

KUDU-1330: Add a tool to unsafely recover from loss of majority replicas

This patch adds an API to allow unsafe config change via an external
recovery tool 'kudu remote_replica replace_config'.

This tool lets us replace a 3-node config on a tablet server with a
1-node config. This is particularly useful when we have 2 out of 3
replicas down and we want to bring the tablet back to operational state.
We can use this tool to force a new config on the surviving node providing
all the details of the new config from the tool. As a result
of the forced config change, the automatic leader election kicks in via
raft mechanisms and the re-replication is triggered from master to bring
the replica count back upto 3-node config.
The lonely survivor talking to the tool tends to become the leader in
new config in majority of the use cases because:
a) The API/tool mimicks the actual leader the survivor node
   had voted for and replicate the new config with a higher term and
   bumped up opid_index. This ensures that 2 new nodes added as part of
   re-replication respect the term emitted by this node and accept
   this node as leader.
b) Assumption is that, the dead nodes are not coming back during this recovery,
   hence the leader chosen in step a) will still be the leader when we see
   the replication factor restored back to 3.

Also, the ReplaceConfig() API adds a way to abort a pending config change
because pending config comes in the way of recovery tool trying to
replicate/commit the new config on the surviving tablet server. There is only
one pending config change allowed at a time for a given tablet, hence
aborting the pending config change seems safest bet.

This patch is a first in series for unsafe config changes, and assumes that
the dead servers are not coming back while the new config change is taking
effect.

TODO:
0) Accept more replica_uuids from the command line to make support multiple
   peers to be added in the new config.
1) Test with a 5-replica config forcing the old {ABCDE} to new {AB} on A.

Change-Id: I908d8c981df74d56dbd034e72001d379fb314700
---
M src/kudu/consensus/consensus.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tserver/tablet_service.cc
15 files changed, 545 insertions(+), 103 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I908d8c981df74d56dbd034e72001d379fb314700
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] [security] introduced TokenSigner::HasValidKey() method

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [security] introduced TokenSigner::HasValidKey() method
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6246/1/src/kudu/security/token_signer.cc
File src/kudu/security/token_signer.cc:

Line 164:   return (tsk_deque_.front()->expire_time() > WallTime_Now());
> I'm a little surprised to see this implementation based on the method name.
Agree: HasValidKey() suggests this method checks for the presence of a valid 
key in the key queue.

I think 'IsCurrentKeyValid' would be a better name.


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

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

[kudu-CR] [catalog manager] categorization of rw operation failures

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [catalog_manager] categorization of rw operation failures
..


Patch Set 8:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/6170/8//COMMIT_MSG
Commit Message:

PS8, Line 10: d/write operation failures happened while executing the leader
: post-election callback
sentence fragment


PS8, Line 14: /
Make it explicit whether this means 'and' or 'or'.


http://gerrit.cloudera.org:8080/#/c/6170/8/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

Line 463:   const Status& s = 
catalog_manager_->ProcessPendingAssignments(to_process);
Would you mind making this an owned Status here and below?  I find the by-value 
return stored as reference pattern to be very unintuitive, and it doesn't match 
what's actually happening.


PS8, Line 763: found
is found


PS8, Line 770:  
the master


PS8, Line 774: already
 : // contain CA certificate information written by the new 
leader master
contain the CA certificate information when this master is re-elected next.


Line 790:   return s;
I'm guessing this is an unnecessary copy because the status is kept as a 
reference.  If it weren't a reference this is pretty much guaranteed NRVO.


PS8, Line 911: const Consensus& consensus
this could just be captured.


Line 914:   const int64_t term_pre =
Why not use 'term'?


Line 938: LOG(WARNING) << op_description << " failed; "
This is logged at INFO level on line 899, why WARNING here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1905 - Allow reinserts on pk only tables

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1905 - Allow reinserts on pk only tables
..

KUDU-1905 - Allow reinserts on pk only tables

Doing a reinsert to a table that as only primary key columns
results in an empty change list. We're currently crashing whenever
we see a empty changelist that is not a delete.

The fix is just to allow empty changelists for reinserts.
This also adds a new flavor of fuzz tests to fuzz-itest.cc
that only have pk-only operations, as well as a directed
regression test that would trigger the problem deterministically.

Ran fuzz-itest in dist-tests with the new tests and the following
command:
KUDU_ALLOW_SLOW_TESTS=1 build-support/dist_test.py --collect-tmpdir \
loop -n 1000 build/debug/bin/fuzz-itest --gtest_repeat=10 \
--gtest_break_on_failure

Tests passed 1000/1000. Results:
http://dist-test.cloudera.org//job?job_id=david.alves.1488580839.22665

Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
---
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_changelist.cc
M src/kudu/integration-tests/fuzz-itest.cc
3 files changed, 105 insertions(+), 29 deletions(-)


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

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

[kudu-CR](branch-1.3.x) KUDU-1906. Fix lost callback for scanner path

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1906. Fix lost callback for scanner path
..


KUDU-1906. Fix lost callback for scanner path

Fixes another case similar to KUDU-1888 in which we were sending an RPC
before setting its deferred. In the case that the RPC responded very
quickly, the response would come before the callback was attached, and
the callback would never get called.

This caused my RowCounter jobs on a small/underpowered test cluster to
have task timeouts a few percent of the time.

This patch fixes the particular instance and also adds some assertions
to try to prevent this style of bug in the future.

Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Reviewed-on: http://gerrit.cloudera.org:8080/6239
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
(cherry picked from commit 7f2624ae4e0132aee24f6b7b2af31e2219ac31f0)
Reviewed-on: http://gerrit.cloudera.org:8080/6253
Reviewed-by: Todd Lipcon 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
4 files changed, 10 insertions(+), 2 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: KUDU-1905 - Allow reinserts on pk only tables

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: WIP: KUDU-1905 - Allow reinserts on pk only tables
..

WIP: KUDU-1905 - Allow reinserts on pk only tables

Doing a reinsert to a table that as only primary key columns
results in an empty change list. We're currently crashing whenever
we see a empty changelist that is not a delete.

The fix is just to allow empty changelists for reinserts.
This also adds a new flavor of fuzz tests to fuzz-itest.cc
that only have pk-only operations, as well as a directed
regression test that would trigger the problem deterministically.

WIP cuz still running dist-tests on this.

Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
---
M src/kudu/common/row_changelist.cc
M src/kudu/integration-tests/fuzz-itest.cc
2 files changed, 103 insertions(+), 23 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7ce03378c7b97fac8ad8cb7783dec4a1b0277344
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 

[kudu-CR] KUDU-1897: disable Kerberos replay cache

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1897: disable Kerberos replay cache
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6254/2/src/kudu/rpc/rpc-negotiation-bench.cc
File src/kudu/rpc/rpc-negotiation-bench.cc:

PS2, Line 109:   // Setup the KDC once. InitKerberosForServer uses global 
state, so it's easier
 :   // not to create a new KDC for every test case.
 :   void SetUp() override {
I think this still sets it up for every test case, doesn't it? maybe use static 
void SetUpTestCase?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbce55a0b12682fdf69e7b2c361c6336495db64d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
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] [security] introduced TokenSigner::HasValidKey() method

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [security] introduced TokenSigner::HasValidKey() method
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6246/1/src/kudu/security/token_signer.cc
File src/kudu/security/token_signer.cc:

Line 164:   return (tsk_deque_.front()->expire_time() > WallTime_Now());
I'm a little surprised to see this implementation based on the method name.  
'HasValidKey' implies to me it will return yes if it has a valid signing key.  
But this is checking that specifically the first key is valid.  Perhaps this 
should be call 'ShouldRotate' or similar?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f85c166c431951d2d676e6936e4bc3b965318c9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] WIP KUDU-1330: Add a tool to unsafely recover from loss of majority replicas

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: WIP KUDU-1330: Add a tool to unsafely recover from loss of 
majority replicas
..


Patch Set 2:

(12 comments)

I am still addressing some refactoring/testing related to David's earlier 
feedbacks, so this update has addressed only Mike's review comments so far.

http://gerrit.cloudera.org:8080/#/c/6066/2//COMMIT_MSG
Commit Message:

PS2, Line 13: perticularly
> particularly
Done


Line 22: a) The API/tool acts as a fake leader mimicking the actual leader the 
node
> As mentioned in comments on raft_consensus.cc the tool should not impersona
I will probably post an update here after a quick testing, is the concern here 
that we may be picking an old leader if we rely on survivor's committed 
config's 'voted_for' field ? Even if we did, since we are bumping up the term, 
we know that this node is going to be elected as leader (assuming dead nodes 
not coming back).


Line 26: a) Assumption is that, the dead nodes are not coming back with a 
higher term,
> s/a/b/
Actually, by 'retained' I meant the the elected leader after election triggered 
as a result of config change in step a). I reworded this now.


Line 43: 2) Add a test case for when the node has a pending config change,
> +1 on this
I hadn't updated this list after my last commit, pls note that some of the test 
cases are already added, I updated the commit msg now to reflect current state.


http://gerrit.cloudera.org:8080/#/c/6066/2/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

PS2, Line 1559: client_cb
> No need to plumb in a callback if this is not an asynchronous method
Done


PS2, Line 1560: error_code
> Would be nice to use this
Done


Line 1600:   consensus_req.set_caller_uuid(leader_uuid);
> This should be the client UUID or a special value to indicate that this was
Do you have a concern on this approach ? Whether survivor is a single follower 
or a single leader, both of them would have had voted for the this leader. This 
approach works in both use cases.  I thought this is a simpler idea than using 
a new/fake uuid from tool. Note that whole change assumes that the dead nodes 
aren't coming back while this is in progress.


Line 1601:   consensus_req.set_caller_term(current_term);
> Use current term + 1
I didn't bump this to single leader use case, because the existing leader steps 
down and bails out from pre-election here without committing a new config: 
https://github.com/apache/kudu/blob/master/src/kudu/consensus/raft_consensus.cc#L401
However, I need to debug little more what other changes are needed if we were 
to bump up the term for single leader survivor. Do you think it's necessary to 
bump up the term for single leader survivor ?


Line 1603:   consensus_req.set_committed_index(last_op_id.index() + 1);
> Don't increment the committed index; leave it at whatever value it previous
Done


PS2, Line 1613: last_op_id.index() + 1
> This is repeated a few times in this function. Let's extract a variable to 
Done


PS2, Line 1615: time_manager_->GetSafeTime().value()
> Hmm. I would like David's input on this
I will poke David here, I don't exactly recall why I had to set here, perhaps 
due a CHECK failure down the line in UpdateReplica. It somewhere has a check 
for timestamp on a consensus request.


http://gerrit.cloudera.org:8080/#/c/6066/2/src/kudu/consensus/time_manager.cc
File src/kudu/consensus/time_manager.cc:

Line 121
> I don't think you need to remove this
In the case of single leader leader propgating the new config, he calls 
UpdateReplica on himself, hence I had to remove couple of DCHECKs as part of 
that. I also removed one on consensus_queue.cc


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I908d8c981df74d56dbd034e72001d379fb314700
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] [kudu-jepsen] install Kudu packages into local repo

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [kudu-jepsen] install Kudu packages into local repo
..

[kudu-jepsen] install Kudu packages into local repo

Install Kudu Java packages into the local maven repository prior to
running kudu-jepsen tests.

Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
---
M src/kudu/scripts/jepsen.sh
1 file changed, 6 insertions(+), 1 deletion(-)


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

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

[kudu-CR] KUDU-1897: disable Kerberos replay cache

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KUDU-1897: disable Kerberos replay cache
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6254/1/src/kudu/rpc/rpc-negotiation-bench.cc
File src/kudu/rpc/rpc-negotiation-bench.cc:

Line 55: using std::bind;
> warning: using decl 'bind' is unused [misc-unused-using-decls]
Done


Line 56: using std::shared_ptr;
> warning: using decl 'shared_ptr' is unused [misc-unused-using-decls]
Done


Line 69: using kudu::security::TokenVerifier;
> warning: using decl 'TokenVerifier' is unused [misc-unused-using-decls]
Done


Line 121: // TODO: this initializes global state that will break if more 
than one test
> warning: missing username/bug in TODO [google-readability-todo]
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbce55a0b12682fdf69e7b2c361c6336495db64d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
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] KUDU-1897: disable Kerberos replay cache

[Dan Burkert (Code Review)]

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

Change subject: KUDU-1897: disable Kerberos replay cache
..

KUDU-1897: disable Kerberos replay cache

Also provides an initial cut of an RPC negotiation benchmark / stress
tester.

Change-Id: Ifbce55a0b12682fdf69e7b2c361c6336495db64d
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/rpc-negotiation-bench.cc
M src/kudu/security/init.cc
3 files changed, 270 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifbce55a0b12682fdf69e7b2c361c6336495db64d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Reserve 1% of disk space by default

[Mike Percy (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: Reserve 1% of disk space by default
..

Reserve 1% of disk space by default

Many people are not aware that Kudu is able to reserve a certain amount
of space per disk for non-Kudu usage and they don't set the
corresponding flags. Let's bump the default from 0 to a special value
indicating one percent, which seems like a reasonable default in most
cases.

Change-Id: I578afeefd9a520fd56bfca597c5fcec8f0f3f98d
---
M src/kudu/consensus/log.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/util/env_util-test.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
5 files changed, 49 insertions(+), 12 deletions(-)


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

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

[kudu-CR] Reserve 1% of disk space by default

[Mike Percy (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: Reserve 1% of disk space by default
..

Reserve 1% of disk space by default

Many people are not aware that Kudu is able to reserve a certain amount
of space per disk for non-Kudu usage and they don't set the
corresponding flags. Let's bump the default from 0 to a special value
indicating one percent, which seems like a reasonable default in most
cases.

Change-Id: I578afeefd9a520fd56bfca597c5fcec8f0f3f98d
---
M src/kudu/consensus/log.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/util/env_util-test.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
5 files changed, 50 insertions(+), 12 deletions(-)


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

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

[kudu-CR] env: Add support for getting FS capacity

[Mike Percy (Code Review)]

Hello Adar Dembo,

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

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

to review the following change.

Change subject: env: Add support for getting FS capacity
..

env: Add support for getting FS capacity

We need a way to get FS capacity in a follow-up patch. This change adds
that capability and changes the Env API to allow for fetching both
capacity and free bytes with a single call. This also allows us to fetch
both values with a single syscall. This API is inspired by
boost::filesystem::space().

This patch also removes the difference in the "free" space returned when
this API is invoked as a superuser vs. a non-privileged user. Now, only
the space available to non-privileged processes is returned. This is
also consistent with the boost API and makes the API more predictable.

Change-Id: Id43275876d3352f5cf943e24ed4513b9f2c131aa
---
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
4 files changed, 43 insertions(+), 30 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id43275876d3352f5cf943e24ed4513b9f2c131aa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR](branch-1.2.x) KUDU-1893 Ensure evaluation of added columns

[David Ribeiro Alves (Code Review)]

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

Change subject: KUDU-1893 Ensure evaluation of added columns
..


KUDU-1893 Ensure evaluation of added columns

During a normal scan, a CFileIterator sets a flag to indicate whether
the block supports decoder-level evaluation, and if, after returning,
this flag has not been set to false, the returned data will be
evaluated against the predicate.

Columns that are added after table creation and after a flush will use
DefaultColumnValueIterators to scan. These iterators were not setting
the flag, so the predicates would not be evaluated on these columns.
All rows in the existing results set would be returned, regardless of
predicates on added columns.

If a column predicate is added to an added column that should filter
out some rows, the scan will yield incorrect results. There is added
test coverage in all_types-scan-correctness-test.cc. Without the
changes in this patch, RunTests() will fail at the following points:
* Null default, Range+IsNotNull
* Null default, Range
* Non-null default, Range+IsNotNull
* Non-null default, Range with Default
* Non-null default, Range without Default

This patch addresses this by forcing predicate evaluation by the
DefaultColumnValueIterator.

Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Reviewed-on: http://gerrit.cloudera.org:8080/6225
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
M src/kudu/common/partial_row.h
M src/kudu/tablet/CMakeLists.txt
A src/kudu/tablet/all_types-scan-correctness-test.cc
6 files changed, 572 insertions(+), 1 deletion(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [catalog manager] categorization of rw operation failures

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [catalog_manager] categorization of rw operation failures
..


Patch Set 8: Verified+1

Looks like a couple of flakes:
  MultiThreadedTabletTest/3.DeleteAndReinsert
  org.apache.kudu.spark.kudu.DefaultSourceTest.Test SparkSQL StringStartsWith 
filters

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1897: disable Kerberos replay cache

[Dan Burkert (Code Review)]

Dan Burkert has uploaded a new change for review.

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

Change subject: KUDU-1897: disable Kerberos replay cache
..

KUDU-1897: disable Kerberos replay cache

Also provides an initial cut of an RPC negotiation benchmark / stress
tester.

Change-Id: Ifbce55a0b12682fdf69e7b2c361c6336495db64d
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/rpc-negotiation-bench.cc
M src/kudu/security/init.cc
3 files changed, 277 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifbce55a0b12682fdf69e7b2c361c6336495db64d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 

[kudu-CR](branch-1.3.x) KUDU-1906. Fix lost callback for scanner path

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1906. Fix lost callback for scanner path
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) KUDU-1906. Fix lost callback for scanner path

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: KUDU-1906. Fix lost callback for scanner path
..

KUDU-1906. Fix lost callback for scanner path

Fixes another case similar to KUDU-1888 in which we were sending an RPC
before setting its deferred. In the case that the RPC responded very
quickly, the response would come before the callback was attached, and
the callback would never get called.

This caused my RowCounter jobs on a small/underpowered test cluster to
have task timeouts a few percent of the time.

This patch fixes the particular instance and also adds some assertions
to try to prevent this style of bug in the future.

Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Reviewed-on: http://gerrit.cloudera.org:8080/6239
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
(cherry picked from commit 7f2624ae4e0132aee24f6b7b2af31e2219ac31f0)
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
4 files changed, 10 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 

[kudu-CR](branch-1.3.x) mapreduce: support for running on secure clusters

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: mapreduce: support for running on secure clusters
..


mapreduce: support for running on secure clusters

This adds the appropriate hooks to grab authentication credentials at
job submission time and add them to the job's Credentials object as a
Hadoop "Token". The tasks then grab the Token and import it into the
client they create before using it.

It's not possible to test this since we don't have support for running
Kerberized Yarn clusters in the MiniCluster environment. I tested
manually on a secure cluster using ImportTsv, ITBLL, and RowCounter
jobs.

Change-Id: Ieed43b9c8646aaee549078a26850e7e7bdecd802
Reviewed-on: http://gerrit.cloudera.org:8080/6237
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
(cherry picked from commit e9dfbe1e5dd38d4f77ab79537e7e5c6d30e56a1d)
Reviewed-on: http://gerrit.cloudera.org:8080/6242
Reviewed-by: Todd Lipcon 
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/CommandLineParser.java
M 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java
M 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableMapReduceUtil.java
M 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableOutputFormat.java
7 files changed, 128 insertions(+), 2 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ieed43b9c8646aaee549078a26850e7e7bdecd802
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) mapreduce: support for running on secure clusters

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: mapreduce: support for running on secure clusters
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieed43b9c8646aaee549078a26850e7e7bdecd802
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [catalog manager] categorization of rw operation failures

[Alexey Serbin (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [catalog_manager] categorization of rw operation failures
..

[catalog_manager] categorization of rw operation failures

This changelist introduces the categorization of the system catalog's
read/write operation failures happened while executing the leader
post-election callback.  There are two categories of errors: fatal and
non-fatal.

If a read/write operation fails in between terms of the system catalog
leadership, the error is considered non-fatal.  In case of a non-fatal
error the leader post-election task bails out: the catalog is no longer
the leader at the original term and the task should be executed by the
new leader upon the ElectedAsLeaderCb.

If a read/write operation failure happened at the same term of the
system catalog leadership, the error is considered fatal and that causes
the master process to crash.  This is to avoid possible inconsistency
when loading into memory and persisting the tables/tablets metadata,
IPKI certificate authority information and TSKs (Token Signing Keys).

All read/write operation failures happened during the system catalog's
shutdown are ignored and the leader post-election task bails out.

The same policy applies to other errors which might happen while
working with IPKI certificate authority information and TokenSigner.
This is because such errors are fatal if they happened to the currently
active system catalog leader, otherwise they can be ignored because
the related information is not used by non-leader catalog manager.
Once the catalog manager becomes a leader again, it will refresh that
information before using it.

Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
5 files changed, 204 insertions(+), 125 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/6170/8
-- 
To view, visit http://gerrit.cloudera.org:8080/6170
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) [security] disable PLAIN SASL mech when authentication is required

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [security] disable PLAIN SASL mech when authentication is 
required
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) [security] disable PLAIN SASL mech when authentication is required

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: [security] disable PLAIN SASL mech when authentication is 
required
..


[security] disable PLAIN SASL mech when authentication is required

Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
Reviewed-on: http://gerrit.cloudera.org:8080/6235
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
(cherry picked from commit 28e9349fb775b0215cd2f7ae9a7531390f79d267)
Reviewed-on: http://gerrit.cloudera.org:8080/6243
---
M src/kudu/rpc/messenger.h
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/server/server_base.cc
5 files changed, 61 insertions(+), 29 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) KUDU-1893 Ensure evaluation of added columns

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1893 Ensure evaluation of added columns
..


KUDU-1893 Ensure evaluation of added columns

During a normal scan, a CFileIterator sets a flag to indicate whether
the block supports decoder-level evaluation, and if, after returning,
this flag has not been set to false, the returned data will be
evaluated against the predicate.

Columns that are added after table creation and after a flush will use
DefaultColumnValueIterators to scan. These iterators were not setting
the flag, so the predicates would not be evaluated on these columns.
All rows in the existing results set would be returned, regardless of
predicates on added columns.

If a column predicate is added to an added column that should filter
out some rows, the scan will yield incorrect results. There is added
test coverage in all_types-scan-correctness-test.cc. Without the
changes in this patch, RunTests() will fail at the following points:
* Null default, Range+IsNull
* Null default, Range+IsNotNull
* Null default, Range
* Non-null default, Range+IsNull
* Non-null default, Range+IsNotNull
* Non-null default, Range with Default
* Non-null default, Range without Default

This patch addresses this by forcing predicate evaluation by the
DefaultColumnValueIterator.

Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Reviewed-on: http://gerrit.cloudera.org:8080/6129
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
(cherry picked from commit 3907340a28bad494cc28a0c93431372a811159d0)
Reviewed-on: http://gerrit.cloudera.org:8080/6240
Reviewed-by: Todd Lipcon 
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
M src/kudu/tablet/all_types-scan-correctness-test.cc
4 files changed, 368 insertions(+), 58 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) KUDU-1834 Don't redact KuduRowResult::ToString()

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1834 Don't redact KuduRowResult::ToString()
..


KUDU-1834 Don't redact KuduRowResult::ToString()

A user on the client side calling ToString on the results of a scan
will likely expect the data to not be redacted. This patch forces
KuduScanPatch::RowPtr::ToString() to not redact.

A test case is added to client-test that turns redaction on and
compares the output of ToString with a redacted row. If the row is
redacted, this case fill fail.

Change-Id: If966e9ba76f747de85350ffa2b91fcf05a9ad324
Reviewed-on: http://gerrit.cloudera.org:8080/6222
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
(cherry picked from commit f1da1eb450cb77878d44d1115d64c05f652378f1)
Reviewed-on: http://gerrit.cloudera.org:8080/6244
Reviewed-by: Todd Lipcon 
---
M src/kudu/client/client-test.cc
M src/kudu/client/scan_batch.cc
2 files changed, 22 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If966e9ba76f747de85350ffa2b91fcf05a9ad324
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) KUDU-1893 Ensure evaluation of added columns

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1893 Ensure evaluation of added columns
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) ITBLL: avoid creating client many times during Generator step

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: ITBLL: avoid creating client many times during Generator step
..


ITBLL: avoid creating client many times during Generator step

Previously ITBLL was creating a new client for each table to be created.
This changes it to create the client at the top of 'run' and reuse the
client.

Change-Id: Ib3d9f5c974e1090b9367784fe8214a21a6b96b77
Reviewed-on: http://gerrit.cloudera.org:8080/6236
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Todd Lipcon 
(cherry picked from commit 2f94fb665ca1e4fb750215f7522d06c5dfb0631f)
Reviewed-on: http://gerrit.cloudera.org:8080/6241
Reviewed-by: Todd Lipcon 
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
1 file changed, 47 insertions(+), 42 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib3d9f5c974e1090b9367784fe8214a21a6b96b77
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.3.x) KUDU-1834 Don't redact KuduRowResult::ToString()

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1834 Don't redact KuduRowResult::ToString()
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If966e9ba76f747de85350ffa2b91fcf05a9ad324
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) ITBLL: avoid creating client many times during Generator step

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: ITBLL: avoid creating client many times during Generator step
..


Patch Set 1: Verified+1

Failure is unrelated (think it's related to 
https://issues.apache.org/jira/browse/KUDU-801 )

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3d9f5c974e1090b9367784fe8214a21a6b96b77
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) ITBLL: avoid creating client many times during Generator step

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: ITBLL: avoid creating client many times during Generator step
..


Patch Set 1: Code-Review+2

just a cherry-pick

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3d9f5c974e1090b9367784fe8214a21a6b96b77
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) mapreduce: support for running on secure clusters

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: mapreduce: support for running on secure clusters
..


Patch Set 1:

this failed because it needs to go on top of the other ITBLL patch on this 
branch. Couldn't figure out a way to cherry-pick the two as a sequence using 
gerrit. will rebase and re-test on top of the other one momentarily.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieed43b9c8646aaee549078a26850e7e7bdecd802
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [catalog manager] categorization of rw operation failures

[Alexey Serbin (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [catalog_manager] categorization of rw operation failures
..

[catalog_manager] categorization of rw operation failures

This changelist introduces the categorization of the system catalog's
read/write operation failures happened while executing the leader
post-election callback.  There are two categories of errors: fatal and
non-fatal.

If a read/write operation fails in between terms of the system catalog
leadership, the error is considered non-fatal.  In case of a non-fatal
error the leader post-election task bails out: the catalog is no longer
the leader at the original term and the task should be executed by the
new leader upon the ElectedAsLeaderCb.

If a read/write operation failure happened at the same term of the
system catalog leadership, the error is considered fatal and that causes
the master process to crash.  This is to avoid possible inconsistency
when loading into memory and persisting the tables/tablets metadata,
IPKI certificate authority information and TSKs (Token Signing Keys).

All read/write operation failures happened during the system catalog's
shutdown are ignored and the leader post-election task bails out.

The same policy applies to other errors which might happen while
working with IPKI certificate authority information and TokenSigner.
This is because such errors are fatal if they happened to the currently
active system catalog leader, otherwise they can be ignored because
the related information is not used by non-leader catalog manager.
Once the catalog manager becomes a leader again, it will refresh that
information before using it.

Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
5 files changed, 213 insertions(+), 125 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/6170/7
-- 
To view, visit http://gerrit.cloudera.org:8080/6170
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [security] introduced TokenSigner::HasValidKey() method

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [security] introduced TokenSigner::HasValidKey() method
..

[security] introduced TokenSigner::HasValidKey() method

Introduced the TokenSigner::HasValidKey() method to check whether
TokenSigner has valid key to sign tokens.  Added a unit test as well.

Change-Id: I2f85c166c431951d2d676e6936e4bc3b965318c9
---
M src/kudu/security/token-test.cc
M src/kudu/security/token_signer.cc
M src/kudu/security/token_signer.h
3 files changed, 39 insertions(+), 1 deletion(-)


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

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

[kudu-CR](branch-1.2.x) KUDU-1893 Ensure evaluation of added columns

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1893 Ensure evaluation of added columns
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1906. Fix lost callback for scanner path

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1906. Fix lost callback for scanner path
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1906. Fix lost callback for scanner path

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: KUDU-1906. Fix lost callback for scanner path
..


KUDU-1906. Fix lost callback for scanner path

Fixes another case similar to KUDU-1888 in which we were sending an RPC
before setting its deferred. In the case that the RPC responded very
quickly, the response would come before the callback was attached, and
the callback would never get called.

This caused my RowCounter jobs on a small/underpowered test cluster to
have task timeouts a few percent of the time.

This patch fixes the particular instance and also adds some assertions
to try to prevent this style of bug in the future.

Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Reviewed-on: http://gerrit.cloudera.org:8080/6239
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
4 files changed, 10 insertions(+), 2 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins