[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 (#6).

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.

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, 187 insertions(+), 120 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/6170/6
-- 
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: 6
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] [catalog manager] proper handling of catalog shutdown

[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 (#5).

Change subject: [catalog_manager] proper handling of catalog shutdown
..

[catalog_manager] proper handling of catalog shutdown

This changelist introduces the categorization of the system catalog's
read/write operation failures happened while executing the leader
post-election task.  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 fails happened at the same term of the system
catalog leadership, the error is considered fatal: this causes the
master process to crash.  This is done do avoid any possible
inconsistency when reading and persisting important system information
such as 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.

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, 187 insertions(+), 120 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/6170/5
-- 
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: 5
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) KUDU-1834 Don't redact KuduRowResult::ToString()

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

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)
---
M src/kudu/client/client-test.cc
M src/kudu/client/scan_batch.cc
2 files changed, 22 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If966e9ba76f747de85350ffa2b91fcf05a9ad324
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Wong 

[kudu-CR] 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
---
M src/kudu/client/client-test.cc
M src/kudu/client/scan_batch.cc
2 files changed, 22 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If966e9ba76f747de85350ffa2b91fcf05a9ad324
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
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 uploaded a new change for review.

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

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)
---
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(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 

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

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

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)
---
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(-)


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

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

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

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

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)
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
1 file changed, 47 insertions(+), 42 deletions(-)


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

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

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

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

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)
---
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(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Wong 

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

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

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
---
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/39/6239/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6239
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
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

[kudu-CR] 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 
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
1 file changed, 47 insertions(+), 42 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Todd Lipcon: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib3d9f5c974e1090b9367784fe8214a21a6b96b77
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] 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 was on an unrelated test

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3d9f5c974e1090b9367784fe8214a21a6b96b77
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] mapreduce: support for running on secure clusters

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

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


Patch Set 1: Code-Review+2

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

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

[kudu-CR] ITBLL: avoid creating client many times during Generator step

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

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


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3d9f5c974e1090b9367784fe8214a21a6b96b77
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
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 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/6239
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 1
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

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

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

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

to review the following change.

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
---
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
3 files changed, 8 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR] WIP [client] retry RPC if authn token expires

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP [client] retry RPC if authn token expires
..

WIP [client] retry RPC if authn token expires

Get a new authn token and retry RPC calls in case of getting
FATAL_INVALID_AUTHENTICATION_TOKEN RPC error.

WIP: need to add more tests.

Change-Id: I20275b4b490145672598631c42824902ebf79ae9
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/scanner-internal.h
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc.cc
M src/kudu/rpc/rpc.h
10 files changed, 210 insertions(+), 69 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20275b4b490145672598631c42824902ebf79ae9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: 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] [util] conventional signature for Status::operator=()

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [util] conventional signature for Status::operator=()
..


[util] conventional signature for Status::operator=()

Updated the signature of the Status::operator=() to be more
conventional.  This fixes TidyBot's warning from
https://gerrit.cloudera.org/#/c/6174/1/src/kudu/util/status.h@403

Change-Id: If04674c88d97204d52bcc15a40755d556f309ea1
Reviewed-on: http://gerrit.cloudera.org:8080/6175
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
Reviewed-by: Todd Lipcon 
---
M src/kudu/util/status.h
1 file changed, 9 insertions(+), 4 deletions(-)

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



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

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

[kudu-CR] [security] disable PLAIN SASL mech when authentication is required

[Dan Burkert (Code Review)]

Dan Burkert 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
---
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/6235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP [client] retry RPC if authn token expires

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: WIP [client] retry RPC if authn token expires
..

WIP [client] retry RPC if authn token expires

Get a new authn token and retry RPC calls in case of getting
FATAL_INVALID_AUTHENTICATION_TOKEN RPC error.

WIP: need to add more tests.

Change-Id: I20275b4b490145672598631c42824902ebf79ae9
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/scanner-internal.h
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc.cc
M src/kudu/rpc/rpc.h
10 files changed, 187 insertions(+), 56 deletions(-)


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

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

[kudu-CR] [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 3: Code-Review+2

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

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

[kudu-CR] [security] disable PLAIN SASL mech when authentication is required

[Dan Burkert (Code Review)]

Hello Todd Lipcon, Kudu Jenkins,

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

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

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

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

[security] disable PLAIN SASL mech when authentication is required

Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
---
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(-)


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

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

[kudu-CR] [security] disable PLAIN SASL mech when authentication is required

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

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


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6235/2/src/kudu/rpc/messenger.cc
File src/kudu/rpc/messenger.cc:

Line 53: #include "kudu/util/net/socket.h"
> is this adding one that was missing?
woops I think this was leftover


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

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

[kudu-CR](branch-1.3.x) server: offer a flag to completely disable the web server

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: server: offer a flag to completely disable the web server
..


server: offer a flag to completely disable the web server

This adds a new --webserver_enabled flag, which can be used to
completely disable the embedded web server. When disabled, the web
server does not listen at all.

The patch required a bit of cleanup to other spots in the code that
required all servers to have associated HTTP addresses. A new unit test
smoke tests a cluster configured in this manner.

Change-Id: I97dacfff075f21cde9248a11fcf19efefb030aec
Reviewed-on: http://gerrit.cloudera.org:8080/6216
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
(cherry picked from commit 9cb5b24f7f957a3ecca1ae1bf034d3ad9defa03a)
Reviewed-on: http://gerrit.cloudera.org:8080/6234
Reviewed-by: Todd Lipcon 
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/master/master.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/thread.cc
M src/kudu/util/thread.h
11 files changed, 100 insertions(+), 46 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I97dacfff075f21cde9248a11fcf19efefb030aec
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](branch-1.3.x) server: offer a flag to completely disable the web server

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: server: offer a flag to completely disable the web server
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97dacfff075f21cde9248a11fcf19efefb030aec
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] [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 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6235/2/src/kudu/rpc/messenger.cc
File src/kudu/rpc/messenger.cc:

Line 53: #include "kudu/util/net/net_util.h"
is this adding one that was missing?


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

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

[kudu-CR] ITBLL: avoid creating client many times during Generator step

[Todd Lipcon (Code Review)]

Hello Dan Burkert, Jean-Daniel Cryans,

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

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

to review the following change.

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
---
M 
java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
1 file changed, 47 insertions(+), 42 deletions(-)


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

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

[kudu-CR] mapreduce: support for running on secure clusters

[Todd Lipcon (Code Review)]

Hello Dan Burkert, Jean-Daniel Cryans,

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

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

to review the following change.

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
---
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(-)


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

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

[kudu-CR] [security] disable PLAIN SASL mech when authentication is required

[Dan Burkert (Code Review)]

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

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

[security] disable PLAIN SASL mech when authentication is required

Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
---
M src/kudu/rpc/messenger.cc
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
6 files changed, 62 insertions(+), 29 deletions(-)


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

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

[kudu-CR] [security] PLAIN SASL mech when authentication is required

[Dan Burkert (Code Review)]

Hello Todd Lipcon, Alexey Serbin,

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

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

to review the following change.

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

[security] PLAIN SASL mech when authentication is required

Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
---
M src/kudu/rpc/messenger.cc
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
6 files changed, 62 insertions(+), 29 deletions(-)


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

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

[kudu-CR](branch-1.3.x) webserver: add X-Frame-Options header

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: webserver: add X-Frame-Options header
..


webserver: add X-Frame-Options header

This adds a default 'DENY' header in order to prevent Kudu web pages
from being put into cross-domain iframes. This can prevent clickjacking
attacks, and generally considered a good idea for web security.

See: https://www.owasp.org/index.php/Clickjacking

Change-Id: Ie43ec476712c2574a4dc746dae6218f0a4195e09
Reviewed-on: http://gerrit.cloudera.org:8080/6215
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
(cherry picked from commit f6a1a60760296e7014d5d7b04ce68d0835721da8)
Reviewed-on: http://gerrit.cloudera.org:8080/6233
Reviewed-by: Todd Lipcon 
---
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/util/curl_util.cc
M src/kudu/util/curl_util.h
4 files changed, 29 insertions(+), 12 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie43ec476712c2574a4dc746dae6218f0a4195e09
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](branch-1.3.x) webserver: add X-Frame-Options header

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: webserver: add X-Frame-Options header
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie43ec476712c2574a4dc746dae6218f0a4195e09
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] [flaky tests] Allow for the master to take longer to start

[David Ribeiro Alves (Code Review)]

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

Change subject: [flaky tests] Allow for the master to take longer to start
..


[flaky tests] Allow for the master to take longer to start

With the additional work that the master has to perform for
security it's taking longer to boot, causing some tests to
timeout while waiting for the master to be ready.
Example from alter_schema_randomized-test:

I0302 03:22:13.666565 10295 catalog_manager.cc:850] Loading CA info into 
memory...
I0302 03:22:14.347914 10295 catalog_manager.cc:894] Loading token signing 
keys...
F0302 03:22:15.072451 20059 alter_table-randomized-test.cc:99] Check failed: 
_s.ok() Bad status: Timed out: Timed out after 30s waiting for master 
(127.0.0.1:46514) startup

This patch just doubles the time we allow the master to
take to boot up before we give up with a Status::TimedOut().

Change-Id: I1389361a3811b265178e3a174bf94af44d40fee9
Reviewed-on: http://gerrit.cloudera.org:8080/6230
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/external_mini_cluster.cc
1 file changed, 1 insertion(+), 1 deletion(-)

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



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

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

[kudu-CR] webserver: add X-Frame-Options header

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: webserver: add X-Frame-Options header
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6215/3/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

Line 70: TAG_FLAG(webserver_x_frame_options, advanced);
> Have a question not specific to the patch, wondering when a flag is tagged 
"advanced" doesn't actually affect anything at runtime, but it does place the 
flags into a different section of the auto-generated flag documentation. 
(Experimental and unsafe flags have the effect of having to be 'unlocked')


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

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

[kudu-CR] webserver: add X-Frame-Options header

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change.

Change subject: webserver: add X-Frame-Options header
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6215/3/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

Line 70: TAG_FLAG(webserver_x_frame_options, advanced);
Have a question not specific to the patch, wondering when a flag is tagged as 
advanced where is the logic to enforce "These flags are for advanced users"?


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

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

[kudu-CR] [util] conventional signature for Status::operator=()

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [util] conventional signature for Status::operator=()
..


Patch Set 3: Code-Review+2

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

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

[kudu-CR] [ts recovery-itest] Reduce the number of rows written in TestChangeMaxCellSize

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [ts_recovery-itest] Reduce the number of rows written in 
TestChangeMaxCellSize
..


Patch Set 2: Code-Review+2

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

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

[kudu-CR] [ts recovery-itest] Reduce the number of rows written in TestChangeMaxCellSize

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: [ts_recovery-itest] Reduce the number of rows written in 
TestChangeMaxCellSize
..


[ts_recovery-itest] Reduce the number of rows written in TestChangeMaxCellSize

This test is failing frequently with a timeout on the scan
(set to 60 secs) because bootstrap of the tablet may take a
long time, e.g:

I0301 23:49:59.183423 25364 ts_tablet_manager.cc:728] T 
bc65854351c541aa858c6483cf78a51b P b7d33205ce694ead90de42aad89ec157: Time spent 
bootstrapping tablet: real 61.689s

The fix is to reduce the amount of rows written so that
boostrap may run faster.

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

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



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

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

[kudu-CR](branch-1.3.x) server: offer a flag to completely disable the web server

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: server: offer a flag to completely disable the web server
..

server: offer a flag to completely disable the web server

This adds a new --webserver_enabled flag, which can be used to
completely disable the embedded web server. When disabled, the web
server does not listen at all.

The patch required a bit of cleanup to other spots in the code that
required all servers to have associated HTTP addresses. A new unit test
smoke tests a cluster configured in this manner.

Change-Id: I97dacfff075f21cde9248a11fcf19efefb030aec
Reviewed-on: http://gerrit.cloudera.org:8080/6216
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
(cherry picked from commit 9cb5b24f7f957a3ecca1ae1bf034d3ad9defa03a)
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/master/master.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/thread.cc
M src/kudu/util/thread.h
11 files changed, 100 insertions(+), 46 deletions(-)


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

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

[kudu-CR] server: offer a flag to completely disable the web server

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: server: offer a flag to completely disable the web server
..


server: offer a flag to completely disable the web server

This adds a new --webserver_enabled flag, which can be used to
completely disable the embedded web server. When disabled, the web
server does not listen at all.

The patch required a bit of cleanup to other spots in the code that
required all servers to have associated HTTP addresses. A new unit test
smoke tests a cluster configured in this manner.

Change-Id: I97dacfff075f21cde9248a11fcf19efefb030aec
Reviewed-on: http://gerrit.cloudera.org:8080/6216
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/master/master.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/server/server_base.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/thread.cc
M src/kudu/util/thread.h
11 files changed, 100 insertions(+), 46 deletions(-)

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



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

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

[kudu-CR] server: offer a flag to completely disable the web server

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: server: offer a flag to completely disable the web server
..


Patch Set 1: Code-Review+2

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

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

[kudu-CR](branch-1.3.x) webserver: add X-Frame-Options header

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: webserver: add X-Frame-Options header
..

webserver: add X-Frame-Options header

This adds a default 'DENY' header in order to prevent Kudu web pages
from being put into cross-domain iframes. This can prevent clickjacking
attacks, and generally considered a good idea for web security.

See: https://www.owasp.org/index.php/Clickjacking

Change-Id: Ie43ec476712c2574a4dc746dae6218f0a4195e09
Reviewed-on: http://gerrit.cloudera.org:8080/6215
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
(cherry picked from commit f6a1a60760296e7014d5d7b04ce68d0835721da8)
---
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/util/curl_util.cc
M src/kudu/util/curl_util.h
4 files changed, 29 insertions(+), 12 deletions(-)


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

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

[kudu-CR] webserver: add X-Frame-Options header

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: webserver: add X-Frame-Options header
..


webserver: add X-Frame-Options header

This adds a default 'DENY' header in order to prevent Kudu web pages
from being put into cross-domain iframes. This can prevent clickjacking
attacks, and generally considered a good idea for web security.

See: https://www.owasp.org/index.php/Clickjacking

Change-Id: Ie43ec476712c2574a4dc746dae6218f0a4195e09
Reviewed-on: http://gerrit.cloudera.org:8080/6215
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/util/curl_util.cc
M src/kudu/util/curl_util.h
4 files changed, 29 insertions(+), 12 deletions(-)

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



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

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

[kudu-CR] webserver: add X-Frame-Options header

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: webserver: add X-Frame-Options header
..


Patch Set 2: Code-Review+2

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

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

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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

Make ExternalDaemon::StartProcess() handle fault injection

external_mini_cluster-test is failing frequently with:
F0302 18:10:29.412564 22826 ts_itest-base.h:487] Check failed: _s.ok() Bad 
status: Runtime error: /tmp/run_tha_testm13fcv/build/debug/bin/kudu-tserver: 
process exited on signal 13

The reason for the failure is that, under load, tests doing fault
injection might fail before StartProcess() completes.
Sometimes these errors get misreported as termination because
of a SIGPIPE signal (number 13).

The fix is to run StartProcess() in loop until it is
successful as long as there is a flag with 'fault' passed
to the daemon.

To reproduce the error I ran this test on dist-test with stress
with the following command:

build-support/dist_test.py loop -n 500 \
build/debug/bin/external_mini_cluster-test \
--stress_cpu_threads=10 --gtest_repeat=10 \
--gtest_break_on_failure

Without this fix the test failed 500/500 times. I inspected
some of the logs and found the same error. The results can
be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488478196.9127

With this fix the test passes 500/500 times. The results
can be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488481249.22125

Change-Id: I6046e34a321de3e324e20e3d63249e4073712447
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
2 files changed, 56 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
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: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [flaky tests] Allow for the master to take longer to start

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [flaky tests] Allow for the master to take longer to start
..


Patch Set 1: Code-Review+2

lgtm, curious to know what's taking so long at boot, maybe we should add some 
more SCOPED_LOG_SLOW_EXECUTION in various spots, but this is also fine.

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

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

[kudu-CR] [flaky tests] Allow for the master to take longer to start

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: [flaky tests] Allow for the master to take longer to start
..

[flaky tests] Allow for the master to take longer to start

With the additional work that the master has to perform for
security it's taking longer to boot, causing some tests to
timeout while waiting for the master to be ready.
Example from alter_schema_randomized-test:

I0302 03:22:13.666565 10295 catalog_manager.cc:850] Loading CA info into 
memory...
I0302 03:22:14.347914 10295 catalog_manager.cc:894] Loading token signing 
keys...
F0302 03:22:15.072451 20059 alter_table-randomized-test.cc:99] Check failed: 
_s.ok() Bad status: Timed out: Timed out after 30s waiting for master 
(127.0.0.1:46514) startup

This patch just doubles the time we allow the master to
take to boot up before we give up with a Status::TimedOut().

Change-Id: I1389361a3811b265178e3a174bf94af44d40fee9
---
M src/kudu/integration-tests/external_mini_cluster.cc
1 file changed, 1 insertion(+), 1 deletion(-)


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

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

[kudu-CR] [util] conventional signature for Status::operator=()

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [util] conventional signature for Status::operator=()
..


Patch Set 3:

> > The symbol name's the same, but does a program built against the
 > > old SO still run properly against the new one?
 > 
 > Yes -- that I verified as well, sure.  Sorry for not mentioning
 > this.
 > I used the following to compile it on MacOS X 10.11.6:
 > 
 > clang++ -shared -o libstatus.so status.cc
 > clang++ main.cc -o main -L . -lstatus
 > 
 > The code in the main.cc:
 > 
 > #include "status.h"
 > 
 > #include 
 > #include 
 > 
 > using namespace std;
 > 
 > int main() {
 > Status s0(0);
 > Status s1(1);
 > 
 > s0 = s1;
 > cout << s0.n() << endl;
 > 
 > Status s2 = 2;
 > s0 = s2;
 > 
 > cout << s0.n() << endl;
 > 
 > return 0;
 > }

And it works if compiled/run at ve0518.halxg.cloudera.com as well.

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

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

[kudu-CR] [util] conventional signature for Status::operator=()

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [util] conventional signature for Status::operator=()
..


Patch Set 3:

> The symbol name's the same, but does a program built against the
 > old SO still run properly against the new one?

Yes -- that I verified as well, sure.  Sorry for not mentioning this.
I used the following to compile it on MacOS X 10.11.6:

clang++ -shared -o libstatus.so status.cc
clang++ main.cc -o main -L . -lstatus

The code in the main.cc:

#include "status.h"

#include 
#include 

using namespace std;

int main() {
  Status s0(0);
  Status s1(1);

  s0 = s1;
  cout << s0.n() << endl;

  Status s2 = 2;
  s0 = s2;

  cout << s0.n() << endl;

  return 0;
}

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

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

[kudu-CR](branch-1.3.x) [security] Add per-connection nonce for Kerberos replay resistance

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
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] Add per-connection nonce for Kerberos replay resistance

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..


[security] Add per-connection nonce for Kerberos replay resistance

Kerberos is susceptible to replay attacks, which it attempts to mitigate
by using a server-side replay cache. The cache is not 100% effective,
and is extremely slow. This commit introduces an effective and efficient
method of mitigating replay attacks by using a server-generated nonce
which the client must send back to the server, wrapped in SASL integrity
protection. This will allow Kudu to disable the replay cache without
negatively affecting security.

No tests are provided, but the codepath is well covered by existing
Kerberos negotiation tests. I intend to write simulated mitm tests to
check this and the channel binding protections soon.

Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Reviewed-on: http://gerrit.cloudera.org:8080/6137
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Todd Lipcon 
(cherry picked from commit ef6e5b58b1ac1425202aaa16dd32bb447f60d814)
Reviewed-on: http://gerrit.cloudera.org:8080/6229
---
M docs/design-docs/rpc.md
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/crypto.h
13 files changed, 254 insertions(+), 106 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
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] [util] conventional signature for Status::operator=()

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [util] conventional signature for Status::operator=()
..


Patch Set 3:

The symbol name's the same, but does a program built against the old SO still 
run properly against the new one?

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

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

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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

Make ExternalDaemon::StartProcess() handle fault injection

external_mini_cluster-test is failing frequently with:
F0302 18:10:29.412564 22826 ts_itest-base.h:487] Check failed: _s.ok() Bad 
status: Runtime error: /tmp/run_tha_testm13fcv/build/debug/bin/kudu-tserver: 
process exited on signal 13

The reason for the failure is that, under load, tests doing fault
injection might fail before StartProcess() completes.
Sometimes these errors get misreported as termination because
of a SIGPIPE signal (number 13).
 
The fix is to run StartProcess() in loop until it is
successful as long as there is a flag with 'fault' passed
to the daemon.

To reproduce the error I ran this test on dist-test with stress
with the following command:

build-support/dist_test.py loop -n 500 \
build/debug/bin/external_mini_cluster-test \
--stress_cpu_threads=10 --gtest_repeat=10 \
--gtest_break_on_failure

Without this fix the test failed 500/500 times. I inspected
some of the logs and found the same error. The results can
be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488478196.9127

With this fix the test passes 500/500 times. The results
can be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488481249.22125

Change-Id: I6046e34a321de3e324e20e3d63249e4073712447
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
2 files changed, 56 insertions(+), 1 deletion(-)


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

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

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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

Make ExternalDaemon::StartProcess() handle fault injection

external_mini_cluster-test is failing frequently with:
F0302 18:10:29.412564 22826 ts_itest-base.h:487] Check failed: _s.ok() Bad 
status: Runtime error: /tmp/run_tha_testm13fcv/build/debug/bin/kudu-tserver: 
process exited on signal 13

The reason for the failure is that, under load, tests doing fault
injection might fail before StartProcess() completes.
Sometimes these errors get misreported as termination because
of a SIGPIPE signal (number 13).

The fix is to run StartProcess() in loop until it is
successful as long as there is a flag with 'fault' passed
to the daemon.

To reproduce the error I ran this test on dist-test with stress
with the following command:

build-support/dist_test.py loop -n 500 \
build/debug/bin/external_mini_cluster-test \
--stress_cpu_threads=10 --gtest_repeat=10 \
--gtest_break_on_failure

Without this fix the test failed 500/500 times. I inspected
some of the logs and found the same error. The results can
be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488478196.9127

With this fix the test passes 500/500 times. The results
can be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488481249.22125

Change-Id: I6046e34a321de3e324e20e3d63249e4073712447
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
2 files changed, 56 insertions(+), 1 deletion(-)


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

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

[kudu-CR] [util] conventional signature for Status::operator=()

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [util] conventional signature for Status::operator=()
..


Patch Set 3: Code-Review+2

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

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

[kudu-CR] Update version to 1.4.0-SNAPSHOT

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Update version to 1.4.0-SNAPSHOT
..


Patch Set 1: Code-Review+2

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

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

[kudu-CR] Update version to 1.4.0-SNAPSHOT

[Jean-Daniel Cryans (Code Review)]

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

Change subject: Update version to 1.4.0-SNAPSHOT
..


Update version to 1.4.0-SNAPSHOT

Change-Id: I128157c46f6c8f77ecee604591619917f5d1ce42
Reviewed-on: http://gerrit.cloudera.org:8080/6223
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M java/interface-annotations/pom.xml
M java/kudu-client-tools/pom.xml
M java/kudu-client/pom.xml
M java/kudu-flume-sink/pom.xml
M java/kudu-jepsen/pom.xml
M java/kudu-mapreduce/pom.xml
M java/kudu-spark/pom.xml
M java/pom.xml
M python/setup.py
M version.txt
10 files changed, 10 insertions(+), 10 deletions(-)

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



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

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

[kudu-CR](branch-1.3.x) [security] Add per-connection nonce for Kerberos replay resistance

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..

[security] Add per-connection nonce for Kerberos replay resistance

Kerberos is susceptible to replay attacks, which it attempts to mitigate
by using a server-side replay cache. The cache is not 100% effective,
and is extremely slow. This commit introduces an effective and efficient
method of mitigating replay attacks by using a server-generated nonce
which the client must send back to the server, wrapped in SASL integrity
protection. This will allow Kudu to disable the replay cache without
negatively affecting security.

No tests are provided, but the codepath is well covered by existing
Kerberos negotiation tests. I intend to write simulated mitm tests to
check this and the channel binding protections soon.

Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Reviewed-on: http://gerrit.cloudera.org:8080/6137
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Todd Lipcon 
(cherry picked from commit ef6e5b58b1ac1425202aaa16dd32bb447f60d814)
---
M docs/design-docs/rpc.md
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/crypto.h
13 files changed, 254 insertions(+), 106 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 

[kudu-CR] [security] Add per-connection nonce for Kerberos replay resistance

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..


[security] Add per-connection nonce for Kerberos replay resistance

Kerberos is susceptible to replay attacks, which it attempts to mitigate
by using a server-side replay cache. The cache is not 100% effective,
and is extremely slow. This commit introduces an effective and efficient
method of mitigating replay attacks by using a server-generated nonce
which the client must send back to the server, wrapped in SASL integrity
protection. This will allow Kudu to disable the replay cache without
negatively affecting security.

No tests are provided, but the codepath is well covered by existing
Kerberos negotiation tests. I intend to write simulated mitm tests to
check this and the channel binding protections soon.

Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Reviewed-on: http://gerrit.cloudera.org:8080/6137
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Todd Lipcon 
---
M docs/design-docs/rpc.md
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/crypto.h
13 files changed, 254 insertions(+), 106 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Gerrit-PatchSet: 11
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 

[kudu-CR] [security] Add per-connection nonce for Kerberos replay resistance

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..


Patch Set 10: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Gerrit-PatchSet: 10
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: No

[kudu-CR] [ts recovery-itest] Reduce the number of rows written in TestChangeMaxCellSize

[David Ribeiro Alves (Code Review)]

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

Change subject: [ts_recovery-itest] Reduce the number of rows written in 
TestChangeMaxCellSize
..

[ts_recovery-itest] Reduce the number of rows written in TestChangeMaxCellSize

This test is failing frequently with a timeout on the scan
(set to 60 secs) because bootstrap of the tablet may take a
long time, e.g:

I0301 23:49:59.183423 25364 ts_tablet_manager.cc:728] T 
bc65854351c541aa858c6483cf78a51b P b7d33205ce694ead90de42aad89ec157: Time spent 
bootstrapping tablet: real 61.689s

The fix is to reduce the amount of rows written so that
boostrap may run faster.

Change-Id: I43a8eec7dd7fd07d880a294a96f189e712e12ebd
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 2 insertions(+), 2 deletions(-)


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

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

[kudu-CR] [ts recovery-itest] Reduce the number of rows written

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: [ts_recovery-itest] Reduce the number of rows written
..

[ts_recovery-itest] Reduce the number of rows written

This test is failing frequently with a timeout on the scan
(set to 60 secs) because bootstrap of the tablet may take a
long time, e.g:

I0301 23:49:59.183423 25364 ts_tablet_manager.cc:728] T 
bc65854351c541aa858c6483cf78a51b P b7d33205ce694ead90de42aad89ec157: Time spent 
bootstrapping tablet: real 61.689s

The fix is to reduce the amount of rows written so that
boostrap may run faster.

Change-Id: I43a8eec7dd7fd07d880a294a96f189e712e12ebd
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 2 insertions(+), 2 deletions(-)


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

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

[kudu-CR] [security] Add per-connection nonce for Kerberos replay resistance

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..


Patch Set 10: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6137/7/src/kudu/security/crypto-test.cc
File src/kudu/security/crypto-test.cc:

PS7, Line 247: kNonceSize, '\0'), nonce
> I don't think it's possible to have a static std::string.
Well, of course the constructor is non-trivial, but the instance would be 
allocated just once during initialization or entering the scope the very first 
time.  However, I agree that it does not make much sense since these tests 
usually executed just once (i.e. almost nobody uses that --gtest-iterations=... 
parameter).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Gerrit-PatchSet: 10
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] Add per-connection nonce for Kerberos replay resistance

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..

[security] Add per-connection nonce for Kerberos replay resistance

Kerberos is susceptible to replay attacks, which it attempts to mitigate
by using a server-side replay cache. The cache is not 100% effective,
and is extremely slow. This commit introduces an effective and efficient
method of mitigating replay attacks by using a server-generated nonce
which the client must send back to the server, wrapped in SASL integrity
protection. This will allow Kudu to disable the replay cache without
negatively affecting security.

No tests are provided, but the codepath is well covered by existing
Kerberos negotiation tests. I intend to write simulated mitm tests to
check this and the channel binding protections soon.

Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
---
M docs/design-docs/rpc.md
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/crypto.h
13 files changed, 254 insertions(+), 106 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Gerrit-PatchSet: 10
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 

[kudu-CR] [security] Add per-connection nonce for Kerberos replay resistance

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..


Patch Set 7:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/6137/7//COMMIT_MSG
Commit Message:

PS7, Line 9: suceptible
> seems to be a typo: susceptible ?
Done


http://gerrit.cloudera.org:8080/#/c/6137/7/docs/design-docs/rpc.md
File docs/design-docs/rpc.md:

PS7, Line 540: tls
> nit: TLS ?
tls-server-end-point is the name defined by the RFC.


http://gerrit.cloudera.org:8080/#/c/6137/7/src/kudu/rpc/sasl_common.cc
File src/kudu/rpc/sasl_common.cc:

Line 351: Status SaslEncode(sasl_conn_t* conn, const std::string& plaintext, 
std::string* encoded) {
> It seems you have just moved this method here, but while you are at it, con
Done


PS7, Line 353:   const char* out;
 :   unsigned out_len;
> Consider making these local for the scope of the while() loop below.
Done


PS7, Line 358: plaintext.size()
> nit: maybe, create a constant for the scope of this method so it would not 
I think this should be hoisted by the compiler, since plaintext is const.


PS7, Line 374:   const char* out;
 :   unsigned out_len;
> Ditto: consider making there local for the while() loop below.
Done


Line 378:   // have to call decode multiple times if our input is larger than 
this max.
> nit: does it make sense to call something like
The plaintext is usually shorter, since there are length delimiters and hash 
overhead.  We're only using these functions for very short tokens, so I'm not 
sure it matters anyway.


http://gerrit.cloudera.org:8080/#/c/6137/7/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:

PS7, Line 788: CHECK_OK(s);
> DCHECK_OK(s) ?  Otherwise, why to have that
Done


http://gerrit.cloudera.org:8080/#/c/6137/7/src/kudu/security/crypto-test.cc
File src/kudu/security/crypto-test.cc:

PS7, Line 246: ASSERT_EQ
> nit here and below: the 'expected' parameter comes first in {ASSERT,EXPECT}
Done


PS7, Line 247: string(kNonceSize, '\0')
> nit: consider making this a constant (or event static constant) for this te
I don't think it's possible to have a static std::string.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Gerrit-PatchSet: 7
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] Add per-connection nonce for Kerberos replay resistance

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..

[security] Add per-connection nonce for Kerberos replay resistance

Kerberos is susceptible to replay attacks, which it attempts to mitigate
by using a server-side replay cache. The cache is not 100% effective,
and is extremely slow. This commit introduces an effective and efficient
method of mitigating replay attacks by using a server-generated nonce
which the client must send back to the server, wrapped in SASL integrity
protection. This will allow Kudu to disable the replay cache without
negatively affecting security.

No tests are provided, but the codepath is well covered by existing
Kerberos negotiation tests. I intend to write simulated mitm tests to
check this and the channel binding protections soon.

Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
---
M docs/design-docs/rpc.md
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/crypto.h
13 files changed, 253 insertions(+), 104 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Gerrit-PatchSet: 9
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 

[kudu-CR] [security] Add per-connection nonce for Kerberos replay resistance

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [security] Add per-connection nonce for Kerberos replay 
resistance
..

[security] Add per-connection nonce for Kerberos replay resistance

Kerberos is suceptible to replay attacks, which it attempts to mitigate
by using a server-side replay cache. The cache is not 100% effective,
and is extremely slow. This commit introduces an effective and efficient
method of mitigating replay attacks by using a server-generated nonce
which the client must send back to the server, wrapped in SASL integrity
protection. This will allow Kudu to disable the replay cache without
negatively affecting security.

No tests are provided, but the codepath is well covered by existing
Kerberos negotiation tests. I intend to write simulated mitm tests to
check this and the channel binding protections soon.

Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
---
M docs/design-docs/rpc.md
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/crypto-test.cc
M src/kudu/security/crypto.cc
M src/kudu/security/crypto.h
13 files changed, 253 insertions(+), 104 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0fb433896963be5e81d349ebf3a044a458e6627
Gerrit-PatchSet: 8
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 

[kudu-CR] [util] conventional signature for Status::operator=()

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [util] conventional signature for Status::operator=()
..


Patch Set 2:

(1 comment)

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

PS2, Line 148: throw();
> I think it prevents something like:
Right, it's needed for 'chaining' assignments like Todd mentioned: it's 
necessary to return reference to the left-hand operand to make it possible 
(returning reference to the right-hand operand is also an option but might 
complicate things in some scenarios).

And yes, as Adar mentioned, it seems to be ABI compatible.  Just in case, I 
played with a small test to verify that:

class Status {
  public:
Status(int n = 0);
//void operator=(const Status& s);
Status& operator=(const Status& s);

int n() const;

  private:
int n_;
};

In both versions for assignment operation the dynamic library compiled had the 
same set of symbols:

aserbin-MBP:op-test$ nm -a libstatus.so.*

libstatus.so.new_signature:
0f50 T __ZN6StatusC1Ei
0f30 T __ZN6StatusC2Ei
0f80 T __ZN6StatusaSERKS_
0fa0 T __ZNK6Status1nEv
 U dyld_stub_binder

libstatus.so.old_signature:
0f50 T __ZN6StatusC1Ei
0f30 T __ZN6StatusC2Ei
0f80 T __ZN6StatusaSERKS_
0fa0 T __ZNK6Status1nEv
 U dyld_stub_binder

The assignment operator does not change its symbol when changing the return 
type:

aserbin-MBP:op-test$ c++filt __ZN6StatusaSERKS_
Status::operator=(Status const&)

It explains why it's not possible to have multiple overloaded function 
signatures different only in return type :)

As for the API compatibility, the new signature of the assignment operator 
broadens the list of expressions where it can be used, so I wouldn't expect 
anything to break from the API perspective.


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

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

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

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change.

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


Patch Set 1:

(7 comments)

Posted differences between this and master:kudu-1893.

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

PS1, Line 24: 
: * Null default, Range
: * Non-null default, Range+IsNotNull
: * Non-null default, Range with Default
: * Non-null default, Range without Default
Removed IsNull test cases


http://gerrit.cloudera.org:8080/#/c/6225/1/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

PS1, Line 515: ) 
Removed "&& !ctx->EvaluatingIsNull()", as IsNull is not in 1.2.x


http://gerrit.cloudera.org:8080/#/c/6225/1/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:

PS1, Line 46: namespace tablet {
: template struct SliceTypeRowOps;
: template struct NumTypeRowOps;
: }  // namespace tablet
This was added in master:kudu-1880, which is not being included in 1.2.x.


Line 486:   template friend struct 
tablet::SliceTypeRowOps;
Same here.


http://gerrit.cloudera.org:8080/#/c/6225/1/src/kudu/tablet/CMakeLists.txt
File src/kudu/tablet/CMakeLists.txt:

Line 99: ADD_KUDU_TEST(all_types-scan-correctness-test)
This was added in master:kudu-1880, which is not being included in 1.2.x.


http://gerrit.cloudera.org:8080/#/c/6225/1/src/kudu/tablet/all_types-scan-correctness-test.cc
File src/kudu/tablet/all_types-scan-correctness-test.cc:

Line 345: int count = 0;
Removed Null default, Range+IsNull case.


Line 395: AddColumn(read_default);
Removed Non-null default, Range+IsNull case.


-- 
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: Kudu Jenkins
Gerrit-HasComments: Yes

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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

Make ExternalDaemon::StartProcess() handle fault injection

external_mini_cluster-test is failing frequently with:
F0302 18:10:29.412564 22826 ts_itest-base.h:487] Check failed: _s.ok() Bad 
status: Runtime error: /tmp/run_tha_testm13fcv/build/debug/bin/kudu-tserver: 
process exited on signal 13

The reason for that is that, under load, test that do fault
injection might fail before StartProcess() completes.
Sometimes these errors get misreported as SIGPIPE (signal
13).

The fix is to run StartProcess() in loop until it is
successful as long as there is a flag with 'fault' passed
to the daemon.

To reproduce the error I ran this test on dist-test with stress
with the following command:

build-support/dist_test.py loop -n 500 \
build/debug/bin/external_mini_cluster-test \
--stress_cpu_threads=10 --gtest_repeat=10 \
--gtest_break_on_failure

Without this fix the test failed 500/500 times. I inspected
some of the logs and found the same error. The results can
be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488478196.9127

With this fix the test passes 500/500 times. The results
can be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488481249.22125

Change-Id: I6046e34a321de3e324e20e3d63249e4073712447
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
2 files changed, 56 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6046e34a321de3e324e20e3d63249e4073712447
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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

Make ExternalDaemon::StartProcess() handle fault injection

external_mini_cluster-test is failing frequently with:
F0302 18:10:29.412564 22826 ts_itest-base.h:487] Check failed: _s.ok() Bad 
status: Runtime error: /tmp/run_tha_testm13fcv/build/debug/bin/kudu-tserver: 
process exited on signal 13

The reason for that is that, under load, test that do fault
injection might fail before StartProcess() completes.
Sometimes these errors get misreported as SIGPIPE (signal
13).

The fix is to run StartProcess() in loop until it is
successful as long as there is a flag with 'fault' passed
to the daemon.

To reproduce the error I ran this test on dist-test with stress
with the following command:

build-support/dist_test.py loop -n 500 \
build/debug/bin/external_mini_cluster-test \
--stress_cpu_threads=10 --gtest_repeat=10 \
--gtest_break_on_failure

Without this fix the test failed 500/500 times. I inspected
some of the logs and found the same error. The results can
be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488478196.9127

With this fix the test passes 500/500 times. The results
can be found at:
http://dist-test.cloudera.org//job?job_id=david.alves.1488481249.22125

Change-Id: I6046e34a321de3e324e20e3d63249e4073712447
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
2 files changed, 57 insertions(+), 1 deletion(-)


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

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

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

[Andrew Wong (Code Review)]

Andrew Wong has uploaded a new change for review.

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

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
---
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(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Andrew Wong 

[kudu-CR] [system catalog] an option to reset CA entries in system table

[Alexey Serbin (Code Review)]

Alexey Serbin has abandoned this change.

Change subject: [system_catalog] an option to reset CA entries in system table
..


Abandoned

If needed, should be done as a separate utility.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1890 Allow renaming of primary key column

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1890 Allow renaming of primary key column
..


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/6078/7/docs/known_issues.adoc
File docs/known_issues.adoc:

PS7, Line 36: * Columns that are part of the primary key can be renamed.
I don't think we need this sentence at all given this is a "limitations" 
section.


http://gerrit.cloudera.org:8080/#/c/6078/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java:

Line 162: // Reopen table for the new schema.
can you add some assertion of what happens if you try scanning with the old 
schema? make sure the exception is reasonable?


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

PS7, Line 3233: that changes dataType of primary column
nit: "Test that changing the data type of a primary key column throws an error"


PS7, Line 3240: Not implemented: cannot support AlterColumn of this type: key2
this isn't a great error message. Can we improve it to be more clear, like 
"cannot alter columns that are part of the primary key"?


http://gerrit.cloudera.org:8080/#/c/6078/7/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

Line 927
see below


PS7, Line 947: 
 : 
I think we should still have some dcheck that the IDs and types match, even if 
the names may have changed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I28a8c52bdb9ac5a3661f9a07c737f7252466d307
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Ram Mettu 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ram Mettu 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] 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+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 
---
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:
  David Ribeiro Alves: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Ram Mettu 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon