[kudu-CR] KUDU-2279 (part 5): don't include entity attributes in log

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9180 )

Change subject: KUDU-2279 (part 5): don't include entity attributes in log
..


Patch Set 3: Code-Review+2

Carrying +2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic995f6a3b8430b73a041de8e5bcbc53a5d8e7f1b
Gerrit-Change-Number: 9180
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Feb 2018 07:59:12 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2279 (part 3): metrics: don't emit untouched metrics to log

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9177 )

Change subject: KUDU-2279 (part 3): metrics: don't emit untouched metrics to log
..

KUDU-2279 (part 3): metrics: don't emit untouched metrics to log

This avoids a big dump of zero-valued metrics at startup.

Change-Id: I92d2c4640e54c91791fab9c420215bafa3fe8f20
Reviewed-on: http://gerrit.cloudera.org:8080/9177
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/server/server_base.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
4 files changed, 68 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I92d2c4640e54c91791fab9c420215bafa3fe8f20
Gerrit-Change-Number: 9177
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-2279 (part 4): enable metrics log by default

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9178 )

Change subject: KUDU-2279 (part 4): enable metrics log by default
..

KUDU-2279 (part 4): enable metrics log by default

Change-Id: Iaeb4c546c2800d8f4288a3626b9e998cd7e35e5c
Reviewed-on: http://gerrit.cloudera.org:8080/9178
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/server/server_base_options.cc
1 file changed, 1 insertion(+), 1 deletion(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaeb4c546c2800d8f4288a3626b9e998cd7e35e5c
Gerrit-Change-Number: 9178
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9176 )

Change subject: KUDU-2279 (part 2): metrics: only emit changed metrics in 
metrics log
..

KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log

This adds a global 'metrics epoch' which can be externally incremented.
When metrics are modified, they remember the epoch of their most recent
modification.

When we dump metrics, we can pass a lower bound in order to see only
metrics which have been modified in or after a given epoch.

This patch updates the metrics logging to only emit metrics that have
changed in each successive line. This should substantially reduce the
size and CPU cost of metric logging on servers with thousands of
tablets.

Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e
Reviewed-on: http://gerrit.cloudera.org:8080/9176
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley 
Reviewed-by: Mike Percy 
---
M src/kudu/server/server_base.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
4 files changed, 141 insertions(+), 26 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Will Berkeley: Looks good to me, but someone else must approve
  Mike Percy: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e
Gerrit-Change-Number: 9176
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-2279 (part 4): enable metrics log by default

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9178 )

Change subject: KUDU-2279 (part 4): enable metrics log by default
..


Patch Set 3: Code-Review+2

carry +2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaeb4c546c2800d8f4288a3626b9e998cd7e35e5c
Gerrit-Change-Number: 9178
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Feb 2018 07:58:50 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2279 (part 3): metrics: don't emit untouched metrics to log

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9177 )

Change subject: KUDU-2279 (part 3): metrics: don't emit untouched metrics to log
..


Patch Set 3: Code-Review+2

carrying +2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92d2c4640e54c91791fab9c420215bafa3fe8f20
Gerrit-Change-Number: 9177
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Feb 2018 07:58:38 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9176 )

Change subject: KUDU-2279 (part 2): metrics: only emit changed metrics in 
metrics log
..


Patch Set 3:

Yea, though we do dump all of the entities even if none of their metrics 
changed so we'll see them disappear when retired.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e
Gerrit-Change-Number: 9176
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Feb 2018 07:58:16 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log

2018-02-05 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9176 )

Change subject: KUDU-2279 (part 2): metrics: only emit changed metrics in 
metrics log
..


Patch Set 3: Code-Review+2

Nice feature. One downside is that it's not possible to tell the difference 
between metrics that were retired (tombstoned tablets) and metrics that haven't 
changed, but in most cases we probably won't care.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e
Gerrit-Change-Number: 9176
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Feb 2018 07:36:59 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

2018-02-05 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8692 )

Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
..


Patch Set 10: Code-Review+1

(2 comments)

This LGTM

http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc
File src/kudu/hms/sasl_client_transport.cc:

http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc@123
PS5, Line 123: ead
> It's probably not possible currently, but exception handling is non-local,
Ok that makes sense. I'm fine with keeping it this way too.

The reason I brought it up was because if some unexpected exception was thrown, 
we'd ideally want to fix it rather than transparently drop it. But I'm fine 
with it either way.


http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc
File src/kudu/hms/sasl_client_transport.cc:

http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc@205
PS7, Line 205:
> Yes, the 'ResetWriteBuf' method restores the prefix, and that's called at t
Oops, missed that. Thanks for the explanation.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
Gerrit-Change-Number: 8692
Gerrit-PatchSet: 10
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Feb 2018 04:34:17 +
Gerrit-HasComments: Yes


[kudu-CR] WIP: Create a rolling-failure endurance test

2018-02-05 Thread Mike Percy (Code Review)
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: WIP: Create a rolling-failure endurance test
..

WIP: Create a rolling-failure endurance test

This script is designed to run for an extended period of time on a
cluster and cause a rolling failure and then restart of all the
kudu-tserver daemons on the cluster. After killing all of the
kudu-tserver processes in random order on a set of tablet servers, the
script will restart all of the processes and then run ksck until the
cluster is healthy again.

The script assumes that it has passwordless sudo access on the local
machine, that the "kudu" administrative tool is in the local PATH, that
it has passwordless sudo access on every machine on the cluster (after
ssh'ing into each machine), and that the starting state is a healthy
cluster with all daemons running.

Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc
---
A src/kudu/scripts/rolling-failure-test.pl
1 file changed, 258 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc
Gerrit-Change-Number: 9222
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] WIP: Create a rolling-failure endurance test

2018-02-05 Thread Mike Percy (Code Review)
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: WIP: Create a rolling-failure endurance test
..

WIP: Create a rolling-failure endurance test

This script is designed to run for an extended period of time on a
cluster and cause a rolling failure and then restart of all the
kudu-tserver daemons on the cluster. After killing all of the
kudu-tserver processes in random order on a set of tablet servers, the
script will restart all of the processes and then run ksck until the
cluster is healthy again.

The script assumes that it has passwordless sudo access on the local
machine, that the "kudu" administrative tool is in the local PATH, that
it has passwordless sudo access on every machine on the cluster (after
ssh'ing into each machine), and that the starting state is a healthy
cluster with all daemons running.

Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc
---
A src/kudu/scripts/rolling-failure-test.pl
1 file changed, 258 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc
Gerrit-Change-Number: 9222
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] WIP: Create a rolling-failure endurance test

2018-02-05 Thread Mike Percy (Code Review)
Hello Alexey Serbin,

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

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

to review the following change.


Change subject: WIP: Create a rolling-failure endurance test
..

WIP: Create a rolling-failure endurance test

This script is designed to run for an extended period of time on a
cluster and cause a rolling failure and then restart of all the
kudu-tserver daemons on the cluster. After killing all of the
kudu-tserver processes in random order on a set of tablet servers, the
script will restart all of the processes and then run ksck until the
cluster is healthy again.

The script assumes that it has passwordless sudo access on the local
machine, that the "kudu" administrative tool is in the local PATH, that
it has passwordless sudo access on every machine on the cluster (after
ssh'ing into each machine), and that the starting state is a healthy
cluster with all daemons running.

Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc
---
A src/kudu/scripts/rolling-failure-test.pl
1 file changed, 0 insertions(+), 0 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc
Gerrit-Change-Number: 9222
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 


[kudu-CR] build-support: fix reporting of build configuration

2018-02-05 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9182 )

Change subject: build-support: fix reporting of build configuration
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6c131f46e75d2eb57d7e0ce9d2ad21e2ad9ab80
Gerrit-Change-Number: 9182
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 06 Feb 2018 03:24:34 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

2018-02-05 Thread Dan Burkert (Code Review)
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Sailesh Mukil, Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
..

KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

The bulk of this commit is adding a new Thrift transport type,
SaslClientTransport, which facilitates SASL GSSAPI negotiation, as well
as integrity/privacy channel protection. The new transport is based on
Impala's version with some significant changes:

- Impala has a client and server SASL transport, necessitating a common
  superclass (SaslTransport). Since we only need a client transport, I
  collapsed all of the logic into a single class, which I think makes the
  code easier to follow.
- The transport uses Kudu helper types where possible, e.g., faststring
  buffers, and our existing SASL utility infrastructure.
- Integrity and privacy channel protection are implemented.

There are no standlone unit-tests for the transport, since that would
require implementing the server-specific counterpart. Instead, the class
is tested indirectly through using the HMS client to communicate with a
Kerberos-enabled HMS instance.

Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
---
M src/kudu/hms/CMakeLists.txt
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/hms_client.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
A src/kudu/hms/sasl_client_transport.cc
A src/kudu/hms/sasl_client_transport.h
M src/kudu/mini-cluster/external_mini_cluster-test.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
14 files changed, 994 insertions(+), 83 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
Gerrit-Change-Number: 8692
Gerrit-PatchSet: 10
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] tablet: don't store row count in delta tracker

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9216 )

Change subject: tablet: don't store row count in delta tracker
..


Patch Set 5:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h
File src/kudu/tablet/diskrowset.h:

http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@366
PS5, Line 366:   Status CountRows(rowid_t *count) const override;
can we mark this final so that calls from within this class can be inlined 
instead of virtual calls?


http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@474
PS5, Line 474:   // Number of rows in the rowset.
can you add a comment here that this might be unset?


http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@475
PS5, Line 475:   mutable std::atomic num_rows_;
let's just use int64_t here, rowid_t is kinda old and gross


http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc
File src/kudu/tablet/diskrowset.cc:

http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@522
PS5, Line 522:   num_rows_(0),
let's use -1 as a signifier of unknown here


http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@672
PS5, Line 672:  rowid_t num_rows;
 :   RETURN_NOT_OK(CountRows(_rows));
this can be inside an #ifndef NDEBUG right?


http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@700
PS5, Line 700:   rowid_t num_rows;
same


http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@725
PS5, Line 725: {
I don't think the extra nesting is worth making the critical section one store 
shorter



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
Gerrit-Change-Number: 9216
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Feb 2018 01:47:29 +
Gerrit-HasComments: Yes


[kudu-CR] Improved logging of DMS flushes

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9204 )

Change subject: Improved logging of DMS flushes
..

Improved logging of DMS flushes

>From compaction-test.cc's TestRowSetInput,

Before:
I0202 14:52:24.983525 2427953984 delta_tracker.cc:727] T test_tablet_id P 
a3e9bdf25f0b431ea57d0249535c29b3: Flushing 20 deltas from DMS 0...
I0202 14:52:24.984046 2427953984 delta_tracker.cc:668] T test_tablet_id P 
a3e9bdf25f0b431ea57d0249535c29b3: Flushed delta block: 0198399763226209 ts 
range: [11, 30]

After:
I0202 14:45:18.924826 2427953984 delta_tracker.cc:726] T test_tablet_id P 
37440651650a4d858267ad408b7e1ac0: Flushing 20 deltas (1759 bytes on disk) from 
DMS 0
I0202 14:45:18.925750 2427953984 delta_tracker.cc:668] T test_tablet_id P 
37440651650a4d858267ad408b7e1ac0: Flushed delta block 0052242700095437 (296 
bytes in memory) stats: ts range=[11, 30], delete_count=[0], 
reinsert_count=[0], update_counts_by_col_id=[12:20,11:20]

Plus one tiny change to clarify logging on flushes.

Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65
Reviewed-on: http://gerrit.cloudera.org:8080/9204
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/tablet.cc
3 files changed, 11 insertions(+), 6 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65
Gerrit-Change-Number: 9204
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] Improved logging of DMS flushes

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9204 )

Change subject: Improved logging of DMS flushes
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65
Gerrit-Change-Number: 9204
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 06 Feb 2018 01:40:34 +
Gerrit-HasComments: No


[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode

2018-02-05 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1953
PS10, Line 1953:   // Make sure that there is no snapshot timestamp sent back.
   :   ASSERT_TRUE(!resp.has_snap_timestamp());
> I'm still unsure why we wouldn't the response to include the timestamp. I a
I see, so looks like you are proposing to make this read mode fault-tolerant as 
well. Which means we need to make it an ordered scan and ensure the client sent 
out the last primary key to retry on.

I do not see any reasons to not make this mode fault tolerant, but I do not see 
the reasons why we have to return the chosen snapshot timestamp for the scan to 
retry on another server in this case. I think the next tsever that the scan is 
retried on can chose the timestamp freely based on the conditions, right?

Moreover, would you mind I do another patch if we do want RYW to be fault 
tolerant, to make it more clear?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 06 Feb 2018 01:21:14 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

2018-02-05 Thread Dan Burkert (Code Review)
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Sailesh Mukil, Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
..

KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

The bulk of this commit is adding a new Thrift transport type,
SaslClientTransport, which facilitates SASL GSSAPI negotiation, as well
as integrity/privacy channel protection. The new transport is based on
Impala's version with some significant changes:

- Impala has a client and server SASL transport, necessitating a common
  superclass (SaslTransport). Since we only need a client transport, I
  collapsed all of the logic into a single class, which I think makes the
  code easier to follow.
- The transport uses Kudu helper types where possible, e.g., faststring
  buffers, and our existing SASL utility infrastructure.
- Integrity and privacy channel protection are implemented.

There are no standlone unit-tests for the transport, since that would
require implementing the server-specific counterpart. Instead, the class
is tested indirectly through using the HMS client to communicate with a
Kerberos-enabled HMS instance.

Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
---
M src/kudu/hms/CMakeLists.txt
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/hms_client.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
A src/kudu/hms/sasl_client_transport.cc
A src/kudu/hms/sasl_client_transport.h
M src/kudu/mini-cluster/external_mini_cluster-test.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
14 files changed, 995 insertions(+), 84 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
Gerrit-Change-Number: 8692
Gerrit-PatchSet: 9
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8692 )

Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc
File src/kudu/hms/sasl_client_transport.cc:

http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc@123
PS5, Line 123: ans
> I didn't realize we could hit non Thrift exceptions in Negotiate(), since S
It's probably not possible currently, but exception handling is non-local, and 
this is more robust to future changes.  Is there a reason to prefer scoping 
down the catch?


http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc@274
PS5, Line 274:
> nvm, I misunderstood the fact that quth-int and auth-conf need to be negoti
I ended up changing the API so that the minimum protection level is passed in 
explicitly; hopefully this makes it a bit more clear (and it reduces some code).


http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc
File src/kudu/hms/sasl_client_transport.cc:

http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc@141
PS7, Line 141:   read_buf_.resize(kFrameHeaderSize);
 :   transport_->readAll(read_buf_.data(), kFrameHeaderSize);
> Why not just read into a uint32_t here and avoid the resize(kFrameHeaderSiz
Done


http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc@205
PS7, Line 205: plaintext.remove_prefix(kFrameHeaderSize);
> Is this correct?
Yes, the 'ResetWriteBuf' method restores the prefix, and that's called at the 
very end of flush, so it should be present on subsequent trips through the loop.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
Gerrit-Change-Number: 8692
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Feb 2018 00:58:53 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8692 )

Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
..


Patch Set 7:

Todd had some offline feedback to give the receive buffer size a flag, which 
necessitated a few refactors, so the latest diff is kinda big.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
Gerrit-Change-Number: 8692
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 06 Feb 2018 00:59:41 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

2018-02-05 Thread Dan Burkert (Code Review)
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Sailesh Mukil, Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
..

KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client

The bulk of this commit is adding a new Thrift transport type,
SaslClientTransport, which facilitates SASL GSSAPI negotiation, as well
as integrity/privacy channel protection. The new transport is based on
Impala's version with some significant changes:

- Impala has a client and server SASL transport, necessitating a common
  superclass (SaslTransport). Since we only need a client transport, I
  collapsed all of the logic into a single class, which I think makes the
  code easier to follow.
- The transport uses Kudu helper types where possible, e.g., faststring
  buffers, and our existing SASL utility infrastructure.
- Integrity and privacy channel protection are implemented.

There are no standlone unit-tests for the transport, since that would
require implementing the server-specific counterpart. Instead, the class
is tested indirectly through using the HMS client to communicate with a
Kerberos-enabled HMS instance.

Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
---
M src/kudu/hms/CMakeLists.txt
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/hms_client.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
A src/kudu/hms/sasl_client_transport.cc
A src/kudu/hms/sasl_client_transport.h
M src/kudu/mini-cluster/external_mini_cluster-test.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/server_negotiation.cc
14 files changed, 987 insertions(+), 84 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345
Gerrit-Change-Number: 8692
Gerrit-PatchSet: 8
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] tablet: don't store row count in delta tracker

2018-02-05 Thread Andrew Wong (Code Review)
Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: tablet: don't store row count in delta tracker
..

tablet: don't store row count in delta tracker

DeltaTracker's constructor previously required a row-count that can
only be obtained by first reading from disk. While useful as a sanity
check while tracking updates, it's not important for this count to be
known at instantiation-time.

This patch is helpful in reducing the amount of data required from disk
at startup, as one of the reasons we need to fully open some CFiles is
to get this row count. This patch itself doesn't defer any reads; it
just defers the requirement of the row count.

This patch has no functional changes.

Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
---
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
5 files changed, 41 insertions(+), 36 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
Gerrit-Change-Number: 9216
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure

2018-02-05 Thread Mahdi Askari (Code Review)
Mahdi Askari has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8960 )

Change subject: Adding a subsection to NTP troubleshooting section to cover 
frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft 
Azure
..


Patch Set 3:

(7 comments)

Comments replied.  Please review.

http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc
File docs/troubleshooting.adoc:

http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@125
PS1, Line 125:  NTP Clock on secure clusters with Centrify
> nit: trailing whitespace
Done


http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@127
PS1, Line 127: - CentrifyDC (Linux package which connects the host to an Active 
Directory) bypasses NTP program.
> nit: can you wrap this to 80-100 coluns as done elsewhere in the file?
Done


http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129
PS1, Line 129:
> can you format file names appropriately using `...`?
Done


http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129
PS1, Line 129:
> this raw link sitting in the documentation doesn't look good
Done


http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129
PS1, Line 129:
> typo
Done


http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129
PS1, Line 129: nk:https:/
> is Centrify Azure-specific? Should this be a separate troubleshooting point
It is not Azure Specific. Can be any cloud or on-prem. I will move it to its 
own section.


http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@135
PS1, Line 135:
> how can this script be automated to run at startup on every startup? Seems
I will have a look and see how we can automate this in start up.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d638aa26137d3a795fce1d0f3f744d4969bdf22
Gerrit-Change-Number: 8960
Gerrit-PatchSet: 3
Gerrit-Owner: Mahdi Askari 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mahdi Askari 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 05 Feb 2018 23:47:30 +
Gerrit-HasComments: Yes


[kudu-CR] tablet: don't store row count in delta tracker

2018-02-05 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9216 )

Change subject: tablet: don't store row count in delta tracker
..


Patch Set 4:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc@1191
PS2, Line 1191:  disk rowsets th
> update comment
Done


http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc@1290
PS2, Line 1290: RETURN_NOT_OK(cur_drs->CountRows(_rows));
> worried about the perf impact here -- this takes a lock and is un-inlinable
Done


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

http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc@530
PS2, Line 530:
> perhaps here we should cache num_rows_ in DiskRowSet
Done


http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc@709
PS2, Line 709:   }
> same, this is a hot function
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
Gerrit-Change-Number: 9216
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 05 Feb 2018 23:36:31 +
Gerrit-HasComments: Yes


[kudu-CR] tablet: don't store row count in delta tracker

2018-02-05 Thread Andrew Wong (Code Review)
Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: tablet: don't store row count in delta tracker
..

tablet: don't store row count in delta tracker

DeltaTracker's constructor previously required a row-count that can
only be obtained by first reading from disk. While useful as a sanity
check while tracking updates, it's not important for this count to be
known at instantiation-time.

This patch is helpful in reducing the amount of data required from disk
at startup, as one of the reasons we need to fully open some CFiles is
to get this row count. This patch itself doesn't defer any reads, it
just defers the requirement of the row count.

This patch has no functional changes.

Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
---
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
5 files changed, 38 insertions(+), 36 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
Gerrit-Change-Number: 9216
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] tablet: don't store row count in delta tracker

2018-02-05 Thread Andrew Wong (Code Review)
Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: tablet: don't store row count in delta tracker
..

tablet: don't store row count in delta tracker

DeltaTracker's constructor previously required a row-count that can
only be obtained by first reading from disk. While useful as a sanity
check while tracking updates, it's not important for this count to be
known at instantiation-time.

This patch is helpful in reducing the amount of data required from disk
at startup, as one of the reasons we need to fully open some CFiles is
to get this row count. This patch itself doesn't defer any reads, it
just defers the requirement of the row count.

This patch has no functional changes.

Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
---
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
5 files changed, 38 insertions(+), 36 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
Gerrit-Change-Number: 9216
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] tablet: don't store row count in delta tracker

2018-02-05 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9216 )

Change subject: tablet: don't store row count in delta tracker
..


Patch Set 2:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc@1191
PS2, Line 1191:  delta trackers
update comment


http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc@1290
PS2, Line 1290: RETURN_NOT_OK(cur_drs->CountRows(_rows));
worried about the perf impact here -- this takes a lock and is un-inlinable, 
and this is the hot path of a compaction. Can we cache the row count somewhere?


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

http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc@530
PS2, Line 530:   RETURN_NOT_OK(DeltaTracker::Open(rowset_metadata_,
perhaps here we should cache num_rows_ in DiskRowSet


http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc@709
PS2, Line 709:   DCHECK_LT(row_idx, num_rows);
same, this is a hot function



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
Gerrit-Change-Number: 9216
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 05 Feb 2018 21:46:10 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns

2018-02-05 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9199 )

Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns
..

KUDU-2281: Add Primary Key Support For INT128 Columns

Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Reviewed-on: http://gerrit.cloudera.org:8080/9199
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke 
---
M src/kudu/client/predicate-test.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_encoder.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partial_row.h
M src/kudu/integration-tests/all_types-itest.cc
10 files changed, 128 insertions(+), 16 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Grant Henke: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Gerrit-Change-Number: 9199
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns

2018-02-05 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9199 )

Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns
..


Patch Set 6:

Added back Dan's +2 after a negligible lint fix.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Gerrit-Change-Number: 9199
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 05 Feb 2018 20:33:39 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns

2018-02-05 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9199 )

Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Gerrit-Change-Number: 9199
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 05 Feb 2018 20:33:17 +
Gerrit-HasComments: No


[kudu-CR] Don't rely on pending config OpId index for peer promotion

2018-02-05 Thread Mike Percy (Code Review)
Mike Percy has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9161 )

Change subject: Don't rely on pending config OpId index for peer promotion
..

Don't rely on pending config OpId index for peer promotion

As part of the effort to make OpId and timestamp assignment atomic
we can't rely on a pending config having been assigned an OpId anymore.
This because the config is marked as the active config _before_ an OpId
is assigned later on, by the queue. The OpId only actually gets set when
the config is committed. Follow up patches will complete remove this
reliance.

In general this doesn't have much impact except where we're promoting a peer.
In this case we currently pass the current active config's term and index from
the queue back to consensus which then uses this info to perform the promotion
only when the current term and index match. We can't do this in the new
setting where the term and index are not available, but moreover this info
is not strictly needed for promotion at all, so this patch removes this
extra info being passed around.

The reasoning why the extra info is not needed is the following:
- If a peer' uuid is marked for promotion in the current committed
config, as verified directly through cmeta, and is considered up-to-date
enough to be promoted, it doesn't matter whether the active config in the
queue is the same as the committed config. The peer still can and should
be promoted.
- Essentially, omitting this extra info just means that we have to re-check
that the member type and attributes are still the expected ones, which is
not costly.

Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Reviewed-on: http://gerrit.cloudera.org:8080/9161
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Mike Percy 
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
4 files changed, 34 insertions(+), 54 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 4
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Don't rely on pending config OpId index for peer promotion

2018-02-05 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9161 )

Change subject: Don't rely on pending config OpId index for peer promotion
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:49:29 +
Gerrit-HasComments: No


[kudu-CR] tablet: don't store row count in delta tracker

2018-02-05 Thread Andrew Wong (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: tablet: don't store row count in delta tracker
..

tablet: don't store row count in delta tracker

DeltaTracker's constructor previously required a row-count that can
only be obtained by first reading from disk. While useful as a sanity
check while tracking updates, it's not important for this count to be
known at instantiation-time.

This patch is helpful in reducing the amount of data required from disk
at startup, as one of the reasons we need to fully open some CFiles is
to get this row count. This patch itself doesn't defer any reads, it
just defers the requirement of the row count.

This patch has no functional changes.

Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
---
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
4 files changed, 22 insertions(+), 30 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
Gerrit-Change-Number: 9216
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] tablet: don't store row count in delta tracker

2018-02-05 Thread Andrew Wong (Code Review)
Andrew Wong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9216


Change subject: tablet: don't store row count in delta tracker
..

tablet: don't store row count in delta tracker

DeltaTracker's constructor previously required a row-count that can
only be obtained by first reading its parent rowset's footer from disk.
While useful as a sanity check while tracking updates, it's not
important for this count to be known at initialization.

This patch is helpful in reducing the amount of data required from disk
at startup, as one of the reasons we need to fully open some CFiles is
to get this row count. This patch itself doesn't defer any reads, it
just defers the requirement of the row count.

This patch has no functional changes.

Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
---
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/diskrowset.cc
4 files changed, 22 insertions(+), 30 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d
Gerrit-Change-Number: 9216
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong 


[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns

2018-02-05 Thread Grant Henke (Code Review)
Hello Tidy Bot, Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns
..

KUDU-2281: Add Primary Key Support For INT128 Columns

Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
---
M src/kudu/client/predicate-test.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_encoder.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partial_row.h
M src/kudu/integration-tests/all_types-itest.cc
10 files changed, 128 insertions(+), 16 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Gerrit-Change-Number: 9199
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9199 )

Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Gerrit-Change-Number: 9199
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:58:52 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns

2018-02-05 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9199 )

Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h
File src/kudu/common/key_encoder.h:

http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h@94
PS3, Line 94:
> It may be better to do static_cast(1) here, so that it d
Done


http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h@116
PS3, Line 116: }
> likewise
Done


http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/integration-tests/all_types-itest.cc
File src/kudu/integration-tests/all_types-itest.cc:

http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/integration-tests/all_types-itest.cc@291
PS3, Line 291: 
builder.AddColumn("decimal32_val")->Type(KuduColumnSchema::DECIMAL)
> Perhaps this should have a decimal column from each size bucket?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Gerrit-Change-Number: 9199
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:47:24 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-721: [Spark] Add DECIMAL type support

2018-02-05 Thread Grant Henke (Code Review)
Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9213


Change subject: KUDU-721: [Spark] Add DECIMAL type support
..

KUDU-721: [Spark] Add DECIMAL type support

Adds DECIMAL support to the Kudu Spark source and RDD.

Change-Id: Ia5f7a801778ed81b68949bbf8d7c08d1a13ed840
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
M 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
M 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala
6 files changed, 84 insertions(+), 25 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia5f7a801778ed81b68949bbf8d7c08d1a13ed840
Gerrit-Change-Number: 9213
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 


[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns

2018-02-05 Thread Grant Henke (Code Review)
Hello Tidy Bot, Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns
..

KUDU-2281: Add Primary Key Support For INT128 Columns

Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
---
M src/kudu/client/predicate-test.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_encoder.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partial_row.h
M src/kudu/integration-tests/all_types-itest.cc
10 files changed, 125 insertions(+), 16 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Gerrit-Change-Number: 9199
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] [docs] Added recommendation to compress PK for backfill inserts

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9185 )

Change subject: [docs] Added recommendation to compress PK for backfill inserts
..

[docs] Added recommendation to compress PK for backfill inserts

Change-Id: I698d954265b4171e4d1bb7e01e286d0d489f1ec7
Reviewed-on: http://gerrit.cloudera.org:8080/9185
Reviewed-by: Dan Burkert 
Tested-by: Dan Burkert 
---
M docs/schema_design.adoc
1 file changed, 64 insertions(+), 18 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved; Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I698d954265b4171e4d1bb7e01e286d0d489f1ec7
Gerrit-Change-Number: 9185
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] [docs] Added recommendation to compress PK for backfill inserts

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has removed a vote on this change.

Change subject: [docs] Added recommendation to compress PK for backfill inserts
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9185
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I698d954265b4171e4d1bb7e01e286d0d489f1ec7
Gerrit-Change-Number: 9185
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] [docs] Added recommendation to compress PK for backfill inserts

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9185 )

Change subject: [docs] Added recommendation to compress PK for backfill inserts
..


Patch Set 4: Verified+1 Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I698d954265b4171e4d1bb7e01e286d0d489f1ec7
Gerrit-Change-Number: 9185
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:27:22 +
Gerrit-HasComments: No


[kudu-CR] [tests] Add more test coverage for INT128 columns

2018-02-05 Thread Grant Henke (Code Review)
Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9193 )

Change subject: [tests] Add more test coverage for INT128 columns
..

[tests] Add more test coverage for INT128 columns

Improves test coverage of reading and writing INT128
columns and fixes a predicate issue exposed by those
tests.

Also adds INT64 tests where it was missing.

Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295
Reviewed-on: http://gerrit.cloudera.org:8080/9193
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/encoding-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/partial_row.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
6 files changed, 69 insertions(+), 11 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295
Gerrit-Change-Number: 9193
Gerrit-PatchSet: 10
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] [docs] Add scaling guide

2018-02-05 Thread Will Berkeley (Code Review)
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: [docs] Add scaling guide
..

[docs] Add scaling guide

This adds some more detailed information on how Kudu scales w.r.t
several resources and provides some background on the scale limits
and how to plan capacity for a Kudu deployment.

Change-Id: I38d8999addc41fe0b726342a27dbba199ddf7dd2
---
A docs/scaling_guide.adoc
1 file changed, 182 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I38d8999addc41fe0b726342a27dbba199ddf7dd2
Gerrit-Change-Number: 8842
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [docs] Add scaling guide

2018-02-05 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8842 )

Change subject: [docs] Add scaling guide
..


Patch Set 2:

New rendering at 
https://github.com/wdberkeley/kudu/blob/scalingdoc/docs/scaling_guide.adoc

Note the "Warning" box shows up as a big exclamation mark when rendered on the 
Kudu website, e.g. 
http://kudu.apache.org/docs/administration.html#_collecting_metrics_to_a_log.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38d8999addc41fe0b726342a27dbba199ddf7dd2
Gerrit-Change-Number: 8842
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:08:55 +
Gerrit-HasComments: No


[kudu-CR] [docs] Add scaling guide

2018-02-05 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8842 )

Change subject: [docs] Add scaling guide
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8842/2/docs/scaling_guide.adoc
File docs/scaling_guide.adoc:

http://gerrit.cloudera.org:8080/#/c/8842/2/docs/scaling_guide.adoc@22
PS2, Line 22: The recommendations and conclusions here are only approximations. 
Appropriate numbers depend on use
: case. There is no substitute for measurement and monitoring of 
resources used during a
: representative workload.
> is there some adoc syntax we can use to make this show up with a big warnin
Done


http://gerrit.cloudera.org:8080/#/c/8842/2/docs/scaling_guide.adoc@88
PS2, Line 88: | Total | 38.5GB
> maybe make two separate rows here -- one for "expected memory usage" and on
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38d8999addc41fe0b726342a27dbba199ddf7dd2
Gerrit-Change-Number: 8842
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:07:51 +
Gerrit-HasComments: Yes


[kudu-CR] [docs] Document how to recover from a majority failed tablet

2018-02-05 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8402 )

Change subject: [docs] Document how to recover from a majority failed tablet
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8402/6/docs/administration.adoc
File docs/administration.adoc:

http://gerrit.cloudera.org:8080/#/c/8402/6/docs/administration.adoc@841
PS6, Line 841: .I
> tiny nit: missing space
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6326f65d029a1cd75e487b16ce5be4baea2f215
Gerrit-Change-Number: 8402
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 05 Feb 2018 17:58:39 +
Gerrit-HasComments: Yes


[kudu-CR] [docs] Document how to recover from a majority failed tablet

2018-02-05 Thread Will Berkeley (Code Review)
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: [docs] Document how to recover from a majority failed tablet
..

[docs] Document how to recover from a majority failed tablet

This adds some docs on how to recover when a tablet can no longer find
a majority due to the permanent failure of replicas.

I tested this procedure by failing tablets in various ways:
- deleting important bits like cmeta or tablet metadata
- deleting entire data dirs
- tombstoning 2/3 replicas (and disabling tombstoned voting)
and I was always able to recover using these instructions.

Change-Id: Ic6326f65d029a1cd75e487b16ce5be4baea2f215
---
M docs/administration.adoc
1 file changed, 65 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic6326f65d029a1cd75e487b16ce5be4baea2f215
Gerrit-Change-Number: 8402
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [tests] Add more test coverage for INT128 columns

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9193 )

Change subject: [tests] Add more test coverage for INT128 columns
..


Patch Set 9: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295
Gerrit-Change-Number: 9193
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 05 Feb 2018 17:50:14 +
Gerrit-HasComments: No


[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9199 )

Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/encoded_key-test.cc
File src/kudu/common/encoded_key-test.cc:

http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/encoded_key-test.cc@178
PS3, Line 178: EXPECT_DECODED_KEY_EQ(INT128, "(int128 
key=170141183460469231731687303715884105727)",
Add a negative value test case.


http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h
File src/kudu/common/key_encoder.h:

http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h@94
PS3, Line 94: uint128_t
It may be better to do static_cast(1) here, so that it 
doesn't use 128bit intermediate types for int8, int32, etc.  Not sure if that 
would cause a performance difference, but seems simpler to just avoid it 
entirely anyways.


http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h@116
PS3, Line 116:   val ^= static_cast(1) << (sizeof(val) * 
CHAR_BIT - 1);
likewise


http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/integration-tests/all_types-itest.cc
File src/kudu/integration-tests/all_types-itest.cc:

http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/integration-tests/all_types-itest.cc@291
PS3, Line 291: 
builder.AddColumn("decimal_val")->Type(KuduColumnSchema::DECIMAL)
Perhaps this should have a decimal column from each size bucket?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd
Gerrit-Change-Number: 9199
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 05 Feb 2018 16:26:28 +
Gerrit-HasComments: Yes


[kudu-CR] [tests] Add more test coverage for INT128 columns

2018-02-05 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9193 )

Change subject: [tests] Add more test coverage for INT128 columns
..


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/cfile-test.cc
File src/kudu/cfile/cfile-test.cc:

http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/cfile-test.cc@563
PS8, Line 563: // Test for BitShuffle builder for UINT8, INT8, UINT16, INT16, 
UINT32, INT32,
> Update this comment.  Also, is the omission of the 64 bit types intentional
Done


http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:

http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/encoding-test.cc@694
PS8, Line 694: // Test for bitshuffle block, for INT32, INT64, INT128, FLOAT, 
DOUBLE
> Same here, seems like 64bit should be added as well.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295
Gerrit-Change-Number: 9193
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 05 Feb 2018 16:20:45 +
Gerrit-HasComments: Yes


[kudu-CR] [tests] Add more test coverage for INT128 columns

2018-02-05 Thread Grant Henke (Code Review)
Hello Tidy Bot, Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: [tests] Add more test coverage for INT128 columns
..

[tests] Add more test coverage for INT128 columns

Improves test coverage of reading and writing INT128
columns and fixes a predicate issue exposed by those
tests.

Also adds INT64 tests where it was missing.

Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/encoding-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/partial_row.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
6 files changed, 69 insertions(+), 11 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295
Gerrit-Change-Number: 9193
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] [tests] Add more test coverage for INT128 columns

2018-02-05 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9193 )

Change subject: [tests] Add more test coverage for INT128 columns
..


Patch Set 8:

(2 comments)

LGTM, but seems like it may be a good opportunity to fill in the 64-bit tests 
as well.

http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/cfile-test.cc
File src/kudu/cfile/cfile-test.cc:

http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/cfile-test.cc@563
PS8, Line 563: // Test for BitShuffle builder for UINT8, INT8, UINT16, INT16, 
UINT32, INT32, FLOAT, DOUBLE
Update this comment.  Also, is the omission of the 64 bit types intentional 
here?  Seems like maybe we overlooked it at some point.


http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:

http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/encoding-test.cc@694
PS8, Line 694: // Test for bitshuffle block, for INT32, INT128, FLOAT, DOUBLE
Same here, seems like 64bit should be added as well.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295
Gerrit-Change-Number: 9193
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 05 Feb 2018 16:08:24 +
Gerrit-HasComments: Yes


[kudu-CR] Improved logging of DMS flushes

2018-02-05 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9204 )

Change subject: Improved logging of DMS flushes
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9204/1/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:

http://gerrit.cloudera.org:8080/#/c/9204/1/src/kudu/tablet/delta_tracker.cc@669
PS1, Line 669: bytes
> can you clarify this as "on disk"
Done


http://gerrit.cloudera.org:8080/#/c/9204/1/src/kudu/tablet/delta_tracker.cc@680
PS1, Line 680:   LOG_WITH_PREFIX(INFO) << "Opened new delta block " << 
block_id.ToString() << " for read";
> I wonder if this log is really useful - seems somewhat redundant with the a
Done


http://gerrit.cloudera.org:8080/#/c/9204/1/src/kudu/tablet/delta_tracker.cc@727
PS1, Line 727: bytes
> can you clarify "in memory" here to contrast from the bytes that we report
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65
Gerrit-Change-Number: 9204
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Mon, 05 Feb 2018 08:47:26 +
Gerrit-HasComments: Yes


[kudu-CR] Improved logging of DMS flushes

2018-02-05 Thread Will Berkeley (Code Review)
Hello Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: Improved logging of DMS flushes
..

Improved logging of DMS flushes

>From compaction-test.cc's TestRowSetInput,

Before:
I0202 14:52:24.983525 2427953984 delta_tracker.cc:727] T test_tablet_id P 
a3e9bdf25f0b431ea57d0249535c29b3: Flushing 20 deltas from DMS 0...
I0202 14:52:24.984046 2427953984 delta_tracker.cc:668] T test_tablet_id P 
a3e9bdf25f0b431ea57d0249535c29b3: Flushed delta block: 0198399763226209 ts 
range: [11, 30]

After:
I0202 14:45:18.924826 2427953984 delta_tracker.cc:726] T test_tablet_id P 
37440651650a4d858267ad408b7e1ac0: Flushing 20 deltas (1759 bytes on disk) from 
DMS 0
I0202 14:45:18.925750 2427953984 delta_tracker.cc:668] T test_tablet_id P 
37440651650a4d858267ad408b7e1ac0: Flushed delta block 0052242700095437 (296 
bytes in memory) stats: ts range=[11, 30], delete_count=[0], 
reinsert_count=[0], update_counts_by_col_id=[12:20,11:20]

Plus one tiny change to clarify logging on flushes.

Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65
---
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/deltafile.h
M src/kudu/tablet/tablet.cc
3 files changed, 11 insertions(+), 6 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65
Gerrit-Change-Number: 9204
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon