[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread William Li (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..

KUDU-1192 Periodically flush glog buffers from a thread

Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.

Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 


[kudu-CR] [catalog manager tsk-itest] shortened test's run-time

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: [catalog_manager_tsk-itest] shortened test's run-time
..


Patch Set 1:

> have you tested this on dist-test with stress? +2ing in case yes
 > but if not I'd prefer we do so as to avoid having to re-fix

I tested it with dist-test using --stress_cpu_threads=8 running 256 iterations 
in DEBUG and TSAN build configurations: 256 out of 256 passed.  Prior to this 
fix ~30 out of 256 failed in both.

Probably, I should run 1K multiples.  Let me do it just to avoid returning to 
this again.

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

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


[kudu-CR] [catalog manager tsk-itest] shortened test's run-time

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: [catalog_manager_tsk-itest] shortened test's run-time
..


Patch Set 1: Code-Review+2

have you tested this on dist-test with stress? +2ing in case yes but if not I'd 
prefer we do so as to avoid having to re-fix

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

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


[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread William Li (Code Review)
William Li has posted comments on this change.

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6853/3/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:

Line 121: active_buf_->flush = true;
> we only want to do this if we haven't flushed in the configured amount of t
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread William Li (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..

KUDU-1192 Periodically flush glog buffers from a thread

Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When wait times out, it will flush even if there is nothing enqueued.

Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 


[kudu-CR] log: fix some incorrect assumptions on BlockingQueue shutdown

2017-05-10 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged.

Change subject: log: fix some incorrect assumptions on BlockingQueue shutdown
..


log: fix some incorrect assumptions on BlockingQueue shutdown

There was an incorrect comment in the Log::AppendThread body in which it
claimed that BlockingQueue::Drain would potentially return false and
also return elements at the same time.

This was in fact not the case. This patch adds unit tests to verify
the actual behavior, and simplifies the code accordingly.

Change-Id: I43e1304ce242f2def26a29504b76bb3e74a93ca3
Reviewed-on: http://gerrit.cloudera.org:8080/6855
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/consensus/log.cc
M src/kudu/util/blocking_queue-test.cc
M src/kudu/util/blocking_queue.h
3 files changed, 35 insertions(+), 10 deletions(-)

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



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

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


[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

2017-05-10 Thread Hao Hao (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 319 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 29
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] log: release WritableLogSegment buffers when log is idle

2017-05-10 Thread Todd Lipcon (Code Review)
Todd Lipcon has abandoned this change.

Change subject: log: release WritableLogSegment buffers when log is idle
..


Abandoned

gonna come back to this in a bit, focusing on the thread reduction thing first 
(in a separate gerrit)

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I512c7f9264ac9490e6a57259e4d7b2608adc1ca7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6853/3/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:

Line 121:   // In case of wait timeout, force it to flush regardless there 
is anything enqueued.
we only want to do this if we haven't flushed in the configured amount of time. 
The way you have it now, we are now flushing every log message.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 
Gerrit-HasComments: Yes


[kudu-CR] log: shut down appender thread when idle

2017-05-10 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: log: shut down appender thread when idle
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6856/1/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

Line 280: return false;
> warning: redundant boolean literal in conditional return statement [readabi
meh, I like it like this, since there is good comment value


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id40a4cfcec96198b537c2f50be7ff1204faf96d2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] faststring: add shrink to fit()

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: faststring: add shrink_to_fit()
..


Patch Set 2: Code-Review+2

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

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


[kudu-CR] log: fix some incorrect assumptions on BlockingQueue shutdown

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: log: fix some incorrect assumptions on BlockingQueue shutdown
..


Patch Set 1: Code-Review+2

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

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


[kudu-CR] log: fix some incorrect assumptions on BlockingQueue shutdown

2017-05-10 Thread Todd Lipcon (Code Review)
Hello Adar Dembo,

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

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

to review the following change.

Change subject: log: fix some incorrect assumptions on BlockingQueue shutdown
..

log: fix some incorrect assumptions on BlockingQueue shutdown

There was an incorrect comment in the Log::AppendThread body in which it
claimed that BlockingQueue::Drain would potentially return false and
also return elements at the same time.

This was in fact not the case. This patch adds unit tests to verify
the actual behavior, and simplifies the code accordingly.

Change-Id: I43e1304ce242f2def26a29504b76bb3e74a93ca3
---
M src/kudu/consensus/log.cc
M src/kudu/util/blocking_queue-test.cc
M src/kudu/util/blocking_queue.h
3 files changed, 35 insertions(+), 10 deletions(-)


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

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


[kudu-CR] log: shut down appender thread when idle

2017-05-10 Thread Todd Lipcon (Code Review)
Hello Adar Dembo,

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

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

to review the following change.

Change subject: log: shut down appender thread when idle
..

log: shut down appender thread when idle

This changes the log appender thread to be based on submitting tasks to a pool,
rather than a thread which is always running.

See the comments on Log::AppendThread in log.cc for notes on the design.

Change-Id: Id40a4cfcec96198b537c2f50be7ff1204faf96d2
---
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/util/blocking_queue-test.cc
M src/kudu/util/blocking_queue.h
M src/kudu/util/threadpool.h
6 files changed, 225 insertions(+), 84 deletions(-)


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

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


[kudu-CR] faststring: add shrink to fit()

2017-05-10 Thread Todd Lipcon (Code Review)
Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: faststring: add shrink_to_fit()
..

faststring: add shrink_to_fit()

This adds a shrink_to_fit() method which reallocates the underlying buffer of a
faststring to match its current length. This is useful in the case where a
faststring acts as a long-lived buffer which occasionally gets large values,
but often contains small ones.

Change-Id: I0e437ff180fccd1957d252fb9a3551bb91ba7917
---
M src/kudu/util/CMakeLists.txt
A src/kudu/util/faststring-test.cc
M src/kudu/util/faststring.cc
M src/kudu/util/faststring.h
4 files changed, 96 insertions(+), 6 deletions(-)


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

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


[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

2017-05-10 Thread Hao Hao (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 319 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [catalog manager tsk-itest] shortened test's run-time

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: [catalog_manager_tsk-itest] shortened test's run-time
..

[catalog_manager_tsk-itest] shortened test's run-time

The longer the test runs the more tables it creates and drops,
and that makes switching from one leader master to another longer.
The presence of some parallel activity exacerbates the problem because
re-elections happen more often.  That leads to timeouts on operations
performed by the client.

This patch shortens the test's run time to make it less flaky if running
on inferior VMs with other concurrent activity.

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


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

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


[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

2017-05-10 Thread Hao Hao (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 315 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: [java-client] Update protoc and simplify the maven build
..

[java-client] Update protoc and simplify the maven build

This patch simplifies the Java build by using maven to
provide protoc via the Maven OS plugin and the
protocArtifact property of the Maven Protobuf plugin.

It also updates protoc to 3.3.0 and makes the required
changes due to compatibility breaks.

Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
---
M docs/installation.adoc
M java/README.md
M java/kudu-client/pom.xml
D 
java/kudu-client/src/main/java/com/google/protobuf/ZeroCopyLiteralByteString.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
M java/pom.xml
15 files changed, 98 insertions(+), 244 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

2017-05-10 Thread Hao Hao (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 315 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 26
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6846/4/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:

Line 235: return value.toByteArray();
> this is resulting in a copy where it wasn't before. I'm a little nervous ab
I will upload a workaround to see if you prefer it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6846/4/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:

Line 235: return value.toByteArray();
this is resulting in a copy where it wasn't before. I'm a little nervous about 
the perf here, but maybe not a big deal since scan results are using sidecars?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1056 and KUDU-1020 Safe time for ksck checksum

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has submitted this change and it was merged.

Change subject: KUDU-1056 and KUDU-1020 Safe time for ksck checksum
..


KUDU-1056 and KUDU-1020 Safe time for ksck checksum

Now that safe time works properly, this patch enables
the snapshot checksum tests for ksck.

Change-Id: Ib45be20dcfa37fb85185302adf84d2c4a55f8c1e
Reviewed-on: http://gerrit.cloudera.org:8080/6843
Reviewed-by: David Ribeiro Alves 
Tested-by: Will Berkeley 
---
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck_remote-test.cc
2 files changed, 20 insertions(+), 28 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Will Berkeley: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib45be20dcfa37fb85185302adf84d2c4a55f8c1e
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 4: Code-Review+2

I'd like at least one other person to take a look.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread William Li (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..

KUDU-1192 Periodically flush glog buffers from a thread

Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
Either gets a signal or wait timeout, it will continue the thread run
and to flush the messages from buffers

Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [java-client] Update protoc and simplify the maven build
..

[java-client] Update protoc and simplify the maven build

This patch simplifies the Java build by using maven to
provide protoc via the Maven OS plugin and the
protocArtifact property of the Maven Protobuf plugin.

It also updates protoc to 3.3.0 and makes the required
changes due to compatibility breaks.

Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
---
M docs/installation.adoc
M java/README.md
M java/kudu-client/pom.xml
D 
java/kudu-client/src/main/java/com/google/protobuf/ZeroCopyLiteralByteString.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
M java/pom.xml
15 files changed, 62 insertions(+), 253 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread William Li (Code Review)
William Li has posted comments on this change.

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..


Patch Set 2:

(2 comments)

removed the changes from other files as well.

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

Line 34: wrapped_(DCHECK_NOTNULL(wrapped)),
> rather than using an env var, should use a gflag for the period at which th
yup. make things much simplier?


Line 54: CHECK_EQ(state_, RUNNING);
> instead of a separate thread, why not just change the existing thread so th
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 
Gerrit-HasComments: Yes


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6846/2/java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
File 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java:

Line 60: pb.setLowerBound(ByteString.copyFrom(this.lowerBound));
> So how come we have to copy these now?
oh wow...we don't. I just commented out for a quick test while mucking around. 
Was not supposed to be included.


http://gerrit.cloudera.org:8080/#/c/6846/2/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java:

Line 264:   // TODO: we could implement a ZeroCopy approach here by 
subclassing LiteralByteString.
> Not sure the comment really makes as much sense now as it did before (since
Done


http://gerrit.cloudera.org:8080/#/c/6846/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java:

Line 26: import org.apache.kudu.PbUtil;
> What's this for?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [java-client] Update protoc and simplify the maven build
..

[java-client] Update protoc and simplify the maven build

This patch simplifies the Java build by using maven to
provide protoc via the Maven OS plugin and the
protocArtifact property of the Maven Protobuf plugin.

It also updates protoc to 3.3.0 and makes the required
changes due to compatibility breaks.

Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
---
M docs/installation.adoc
M java/README.md
M java/kudu-client/pom.xml
D 
java/kudu-client/src/main/java/com/google/protobuf/ZeroCopyLiteralByteString.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
M java/pom.xml
15 files changed, 63 insertions(+), 253 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread William Li (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..

KUDU-1192 Periodically flush glog buffers from a thread

Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
Either gets a signal or wait timeout, it will continue the thread to flush the 
messages from buffers

Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6846/2/java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
File 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java:

Line 60: pb.setLowerBound(ByteString.copyFrom(this.lowerBound));
So how come we have to copy these now?


http://gerrit.cloudera.org:8080/#/c/6846/2/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java:

Line 264:   // TODO: we could implement a ZeroCopy approach here by 
subclassing LiteralByteString.
Not sure the comment really makes as much sense now as it did before (since 
before it was implied that ZeroCopyLiteralByteString subclasses 
LiteralByteString, and UnsafeByteOperations doesn't do that).


http://gerrit.cloudera.org:8080/#/c/6846/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java:

Line 26: import org.apache.kudu.PbUtil;
What's this for?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] WIP Allow external miniclusters to use many data dirs

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: WIP Allow external miniclusters to use many data dirs
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6845/1/src/kudu/integration-tests/external_mini_cluster.h
File src/kudu/integration-tests/external_mini_cluster.h:

Line 70:   // Number of directories to create for each daemon.
> Hmm, the ext cluster currently is organized as:
Yeah, that's more or less what I had in mind.

Agreed that the postfixes seem cosmetic at best; may not be worth the added 
code.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2f5def6980ad394c8558ad97ba830f1b0257332
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] log: release WritableLogSegment buffers when log is idle

2017-05-10 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: log: release WritableLogSegment buffers when log is idle
..


Patch Set 2:

ok, let me see if I can rework this and just bite the bullet on the thread 
shutdown too

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I512c7f9264ac9490e6a57259e4d7b2608adc1ca7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] faststring: add shrink to fit()

2017-05-10 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: faststring: add shrink_to_fit()
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6835/1/src/kudu/util/faststring-test.cc
File src/kudu/util/faststring-test.cc:

PS1, Line 33: TestShrinkToFit_SmallerThanInitialCapacity
> nit: TestShrinkToFit_DontShrinkIfSmallerThanInitialCapacity ? or just expla
Done


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

PS1, Line 208:   // Any pointers within this instance are invalidated.
> mention that this doesn't shrink below the initial capacity
Done


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

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


[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

2017-05-10 Thread Hao Hao (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 315 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 25
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

2017-05-10 Thread Hao Hao (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However network access is
not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 315 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 24
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6846/1/java/kudu-client/pom.xml
File java/kudu-client/pom.xml:

Line 138: 
com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
> Will test and report back.
I ran a `mvn clean install -DskipTests` on each of these without issue.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] WIP Allow external miniclusters to use many data dirs

2017-05-10 Thread Andrew Wong (Code Review)
Andrew Wong has posted comments on this change.

Change subject: WIP Allow external miniclusters to use many data dirs
..


Patch Set 1:

(1 comment)

Even the most cursory glance is greatly appreciated!

http://gerrit.cloudera.org:8080/#/c/6845/1/src/kudu/integration-tests/external_mini_cluster.h
File src/kudu/integration-tests/external_mini_cluster.h:

Line 70:   // Number of directories to create for each daemon.
> Wouldn't it be more natural to convert data_root into e.g. data_roots and a
Hmm, the ext cluster currently is organized as:
/test-path
+-cluster (this level is what is included in data_root)
  +-daemon (master, ts-0, ts-1)
+- data dir (now supports multiple: data-0, data-1 via num_dirs_per_tserver)
  +- wals, data, etc.

I suppose we could specify multiple directories for each cluster, have each 
directory mirror each other's layout, and spread data that way.
e.g.
/cluster-0/ts-0/data/{wals,data,etc},
/cluster-1/ts-0/data/data,
/cluster-2/ts-0/data/data

Where cluster-i is the ith disk on a cluster. Seems a bit complex, but good 
point that it could be used to test actual multi-disk systems. Is that what 
you're envisioning?

I could see replacing num_dirs_per_tserver with a list of path posfixes, 
although this would only serve as an aesthetic bump, if anything.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2f5def6980ad394c8558ad97ba830f1b0257332
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..


Patch Set 1:

(3 comments)

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

Line 34: const char* kAsyncLoggerTimerEnvVar = "KUDU_ASYNC_LOGGER_TIMER";
rather than using an env var, should use a gflag for the period at which this 
is flushed. We can probably just reuse the existing FLAGS_logbufsecs in fact


Line 54:   timer_thread_ = std::thread(&AsyncLogger::RunTimer, this);
instead of a separate thread, why not just change the existing thread so that 
instead of doing a Wait() with no timeout, it does a Wait() with a timeout of 
logbufsecs and then wakes up and flushes even if nothing has been enqueued?


Line 148:   wake_flusher_cond_.Wait();
eg here you can use a TimedWait, and if it times out, set active_buf_->flush = 
true


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

2017-05-10 Thread Hao Hao (Code Review)
Hao Hao has posted comments on this change.

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..


Patch Set 22:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/6514/22//COMMIT_MSG
Commit Message:

PS22, Line 13: adavance
> typo: advanced
Done


PS22, Line 17: However network
> "However, if"
Done


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

Line 82:   kudu::g_trusted_subnets->push_back(network);
> I think something like:
Done


PS22, Line 85: can only be interpreted as
> "must be specified in"
Done


http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/rpc/server_negotiation.h
File src/kudu/rpc/server_negotiation.h:

Line 42: extern vector* g_trusted_subnets;
> hm, where is this used externally? we try to avoid globals with such wide s
Done


http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/util/net/net_util-test.cc
File src/kudu/util/net/net_util-test.cc:

PS22, Line 95: TestPrivateAddresses
> rename to TestWithinNetwork (this isn't specific to public/private)
Done


http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:

PS22, Line 186: string Network::ToString() const {
  :   return Substitute("$0:$1", addr_, mask_);
  : }
> this ToString isn't really human-readable. I think it would be better to ei
Done


Line 252: Status GetLocalNetwork(std::vector* net) {
> nit: rename param to 'nets' since it is a list, not a single one
Done


PS22, Line 256:   auto cleanup = MakeScopedCleanup([&]() {
  : freeifaddrs(ifap);
  :   });
> this should probably be moved below the error check, otherwise we may freei
Done


Line 265: 
> should probably 'net->clear()' here, since the docs say it gets them, not a
Done


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

Line 98:   Network(uint32_t addr, uint32_t mask);
> add doc here regarding endianness, etc.
Done


PS22, Line 107: ParseString
> rename to ParseCIDR so it's clear this is a CIDR style string and not a net
Done


PS22, Line 111: mask_
> I think conventionally people refer to this as 'netmask' instead of 'mask'.
Done


PS22, Line 131: GetLocalNetwork
> since this gives back a list, change to GetLocalNetworks
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs

2017-05-10 Thread Hao Hao (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
..

KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs

This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.

An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However network access is
not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.

Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 317 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: [java-client] Update protoc and simplify the maven build
..

[java-client] Update protoc and simplify the maven build

This patch simplifies the Java build by using maven to
provide protoc via the Maven OS plugin and the
protocArtifact property of the Maven Protobuf plugin.

It also updates protoc to 3.3.0 and makes the required
changes due to compatibility breaks.

Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
---
M docs/installation.adoc
M java/README.md
M java/kudu-client/pom.xml
D 
java/kudu-client/src/main/java/com/google/protobuf/ZeroCopyLiteralByteString.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
M java/pom.xml
15 files changed, 64 insertions(+), 252 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6846/1/java/kudu-client/pom.xml
File java/kudu-client/pom.xml:

Line 144
> To remove this, I think you need to add attachProtoSources=false to the pro
I think this step was just trying to remove the test proto's due to some 
unusual behavior. See "**/*test*.proto".

I tested and no test proto is included after my change. Both before and after 
this change 25 kudu proto files are included in the final jar. I also verified 
with a jar in maven central.


Line 138: 
com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
> If you don't mind please try this out on SLES 12 SP1, Red Hat or CentOS 6.6
Will test and report back.


http://gerrit.cloudera.org:8080/#/c/6846/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java:

Line 30: import com.google.protobuf.UnsafeByteOperations;
> duplicate
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [util] updated output from TryRunLsof()

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged.

Change subject: [util] updated output from TryRunLsof()
..


[util] updated output from TryRunLsof()

TryRunLsof() should not conclude it's called because of an error
binding to the specified address: in some use-cases it might not be the
case.  The output is updated to remove the statement about a failure to
bind to the specified address, leaving just the output from lsof.

This is a pure cosmetic fix to improve error message readability
in the context of KUDU-2005.

Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Reviewed-on: http://gerrit.cloudera.org:8080/6852
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/util/net/net_util.cc
1 file changed, 3 insertions(+), 4 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] log: release WritableLogSegment buffers when log is idle

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: log: release WritableLogSegment buffers when log is idle
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6611/3/src/kudu/consensus/log.h
File src/kudu/consensus/log.h:

PS3, Line 99: Status SwitchToAsyncMode();
> this new state change seems weird because it's one way. could we just refac
My issue with an explicit "async mode" is that it exposes the append thread's 
existence (or non-existence) outside this class. To reduce our overall thread 
count, an "idle" append thread should exit and a future async operation should 
start it up again, but that becomes harder to do if the append thread's 
existence spills out into other classes via this "async mode" thing. I guess if 
there were a "switch back to sync mode" function then it'd be OK, but ideally 
this class would manage the starting/stopping of the append thread 
automatically, without forcing its users to do it.

Incidentally, did you explore having the append thread exit outright on idle, 
and bringing it back up on the next async operation? The synchronization gets 
hairy...


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

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


[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

2017-05-10 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..


Patch Set 2:

(2 comments)

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

Line 152: using kudu::consensus::OpId;
> warning: using decl 'OpId' is unused [misc-unused-using-decls]
Done


Line 153: using kudu::consensus::MaximumOpId;
> warning: using decl 'MaximumOpId' is unused [misc-unused-using-decls]
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

2017-05-10 Thread Will Berkeley (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..

KUDU-2001 Add UNDO size to tablet on-disk size

To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counts
REDOs and is used for compaction scoring.

Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
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
M src/kudu/tablet/tablet.cc
5 files changed, 25 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] Refactor ConsensusStatePB to hold committed and pending configs

2017-05-10 Thread Mike Percy (Code Review)
Mike Percy has posted comments on this change.

Change subject: Refactor ConsensusStatePB to hold committed and pending configs
..


Patch Set 6:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:

PS6, Line 103: There is a corner case in Raft where a node may be elected 
leader of a
 :   // pending (uncommitted) configuration. In such a case, the 
master will
 :   // detect that the node is not a member of the committed 
configuration.
I think you can just say: The leader may be a part of the committed or the 
pending configuration (or both).

What the master does with that information isn't part of the contract of this 
data structure.


PS6, Line 113: 5
4


http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/consensus/quorum_util.cc
File src/kudu/consensus/quorum_util.cc:

PS6, Line 195:  
nit: stray space here


PS6, Line 199:   
nit: funny extra indentation


Line 219: string DiffConsensusStates(const ConsensusStatePB& old_state,
Looks like this needs to detect differences in pending configs now too.


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

PS6, Line 2235:   
nit: indent at least an extra 4 spaces for a line wrap


PS6, Line 2362: Previously, this field was omitted from the
  : // consensus state in this situation.
I don't think this helps clarify this code, consider removing this part of the 
comment.


http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

Line 134: RETURN_NOT_OK(consensus::VerifyConsensusState(cstate));
Add something like CHECK(!cstate.has_pending_config()) since we should never 
persist a pending config.


http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/tserver/tablet_copy.proto
File src/kudu/tserver/tablet_copy.proto:

PS6, Line 116: committed 
remove this word


http://gerrit.cloudera.org:8080/#/c/6809/6/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS6, Line 951: *reported_tablet->mutable_consensus_state() =
 : consensus->ConsensusState();
nit: This will fit on one line now


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

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


[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread

2017-05-10 Thread William Li (Code Review)
William Li has uploaded a new change for review.

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

Change subject: KUDU-1192 Periodically flush glog buffers from a thread
..

KUDU-1192 Periodically flush glog buffers from a thread

Added a timer inside the AsyncLogger to periodically calls the flush().
It wakes up the main RunThread() to flush the messages from buffers
Period to wake up is default to 1 second but can also use environment variable 
to change it

Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/async_logger.h
M src/kudu/util/logging-test.cc
3 files changed, 113 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 


[kudu-CR] [util] updated output from TryRunLsof()

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: [util] updated output from TryRunLsof()
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] [util] updated output from TryRunLsof()

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: [util] updated output from TryRunLsof()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6852/1/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:

PS1, Line 293: if 
> Nit: it was more grammatically correct without 'if'.
Done


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

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


[kudu-CR] [util] updated output from TryRunLsof()

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new patch set (#2).

Change subject: [util] updated output from TryRunLsof()
..

[util] updated output from TryRunLsof()

TryRunLsof() should not conclude it's called because of an error
binding to the specified address: in some use-cases it might not be the
case.  The output is updated to remove the statement about a failure to
bind to the specified address, leaving just the output from lsof.

This is a pure cosmetic fix to improve error message readability
in the context of KUDU-2005.

Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
---
M src/kudu/util/net/net_util.cc
1 file changed, 3 insertions(+), 4 deletions(-)


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

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


[kudu-CR] [util] updated output from TryRunLsof()

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: [util] updated output from TryRunLsof()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6852/1/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:

PS1, Line 293: if 
Nit: it was more grammatically correct without 'if'.


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

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


[kudu-CR] [util] updated output from TryRunLsof()

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: [util] updated output from TryRunLsof()
..

[util] updated output from TryRunLsof()

TryRunLsof() should not conclude it's called because of an error
binding to the specified address: in some use-cases it might not be the
case.  The output is updated to remove the statement about a failure to
bind to the specified address, leaving just the output from lsof.

This is a pure cosmetic fix to improve error message readability
in the context of KUDU-2005.

Change-Id: I1ff0d28f631c7f392420040b132f7542f08b74d6
---
M src/kudu/util/net/net_util.cc
1 file changed, 3 insertions(+), 4 deletions(-)


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

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


[kudu-CR] KUDU-1941: more validation for RPC auth flags

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged.

Change subject: KUDU-1941: more validation for RPC auth flags
..


KUDU-1941: more validation for RPC auth flags

With this patch, both master and tserver refuse to start if
authentication is 'required' but no authentication method is configured.

Prior to this patch, the inconsistency with the run-time configuration
could be detected at a later stage when a client would try to connect
to Kudu cluster.

Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
Reviewed-on: http://gerrit.cloudera.org:8080/6851
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/rpc/messenger.cc
1 file changed, 11 insertions(+), 2 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
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: Todd Lipcon 


[kudu-CR] KUDU-1941: more validation for RPC auth flags

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: KUDU-1941: more validation for RPC auth flags
..


Patch Set 1: Code-Review+2

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

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


[kudu-CR] KUDU-2005: actionable error messages from webserver

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has submitted this change and it was merged.

Change subject: KUDU-2005: actionable error messages from webserver
..


KUDU-2005: actionable error messages from webserver

As it turned out, squeasel outputs only errors via the log_message
callback.  Let's output them with ERROR severity into Kudu log
(they were output as INFO messages prior to this patch).

Also, if the embedded squeasel webserver fails to start, add the last
error message from it (if any) into the RuntimeError status message
returned from Webserver::Start().

Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Reviewed-on: http://gerrit.cloudera.org:8080/6848
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley 
---
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
3 files changed, 25 insertions(+), 6 deletions(-)

Approvals:
  Will Berkeley: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
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: Will Berkeley 


[kudu-CR] KUDU-1941: more validation for RPC auth flags

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: KUDU-1941: more validation for RPC auth flags
..

KUDU-1941: more validation for RPC auth flags

With this patch, both master and tserver refuse to start if
authentication is 'required' but no authentication method is configured.

Prior to this patch, the inconsistency with the run-time configuration
could be detected at a later stage when a client would try to connect
to Kudu cluster.

Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
---
M src/kudu/rpc/messenger.cc
1 file changed, 11 insertions(+), 2 deletions(-)


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

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


[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

2017-05-10 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..


Patch Set 1:

(1 comment)

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

Line 151: using base::subtle::Barrier_AtomicIncrement;
> warning: using decl 'Barrier_AtomicIncrement' is unused [misc-unused-using-
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1056 and KUDU-1020 Safe time for ksck checksum

2017-05-10 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-1056 and KUDU-1020 Safe time for ksck checksum
..


Patch Set 3: Verified+1

-raft_consensus-itest failure is a flake I've seen a lot on master
-MultiThreadedTabletTest appears to be a rare known failure that is unrelated 
to the patch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib45be20dcfa37fb85185302adf84d2c4a55f8c1e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

2017-05-10 Thread Will Berkeley (Code Review)
Will Berkeley has uploaded a new patch set (#2).

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..

KUDU-2001 Add UNDO size to tablet on-disk size

To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counts
REDOs and is used for compaction scoring.

Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
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
M src/kudu/tablet/tablet.cc
5 files changed, 25 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-2005: actionable error messages from webserver

2017-05-10 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-2005: actionable error messages from webserver
..


Patch Set 1: Code-Review+2

(1 comment)

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

PS1, Line 246: TryRunLsof(addr);
> I'm not sure I understand -- the error message below is generated from sque
Thanks for looking into the squeasel code to see if we could do better re: lsof 
check. I agree it's best not to parse the message.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Gerrit-PatchSet: 1
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: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-2001 Add UNDO size to tablet on-disk size

2017-05-10 Thread Will Berkeley (Code Review)
Will Berkeley has uploaded a new change for review.

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

Change subject: KUDU-2001 Add UNDO size to tablet on-disk size
..

KUDU-2001 Add UNDO size to tablet on-disk size

To make the on-disk size metric more accurate, this
patch makes DeltaTracker::EstimateOnDiskSize() count
UNDO deltas and adds a new method
DeltaTracker::EstimateOnDiskSizeForCompaction() that
retains the original behavior, which only counted
REDOs and was used for compaction scoring.

Change-Id: I59001adadb9a768a464e7b2cf0f0a5df0ef5393a
---
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
M src/kudu/tablet/tablet.cc
5 files changed, 25 insertions(+), 7 deletions(-)


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

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


[kudu-CR] KUDU-2005: actionable error messages from webserver

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

Change subject: KUDU-2005: actionable error messages from webserver
..


Patch Set 1:

(1 comment)

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

PS1, Line 246: TryRunLsof(addr);
> Put this after the error message below, since the below puts it in context.
I'm not sure I understand -- the error message below is generated from squeasel 
error message and the http_address_.  TryRunLsof() just outputs its messages 
into the log, since the second argument of TryRunLsof() is empty.  Also, that 
output goes into the WARNING log file, not ERROR one.

I looked into the squeasel source code and I could not find the way to detect 
that condition if not parsing the error message returned from it.

That's the excerpt:

cry(fc(ctx), "%s: cannot bind to %.*s: %d (%s)", __func__,
  (int) vec.len, vec.ptr, ERRNO, strerror(errno));

So, I would rather leave it as is for now since I'm not a fan of parsing error 
messages.  Expecting/checking for particular error message patterns in tests is 
more or less tolerable, but not here.  There are too many risks: the squeasel 
code can change, that particular error message might be not the last one we 
receive, etc.

Let's change the output from TryRunLsof() to remove the ' "Failed to bind to " 
<< addr.ToString() << ". "' part -- that would be less confusion to the readers 
(I will post that patch separately).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Gerrit-PatchSet: 1
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: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1549: delete dead LBM containers at startup

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged.

Change subject: KUDU-1549: delete dead LBM containers at startup
..


KUDU-1549: delete dead LBM containers at startup

Full containers with no live blocks are "dead" in that they will never be
used for any block operations. These containers are safe to delete, and
though we lose some forensic information (e.g. block X was created at time Y
and deleted at time Z) in doing so, we'll wind up processing less container
metadata in the next startup.

This patch adds a quick and dirty implementation of dead container deletion
at startup. It would be nice to also do it in real time (perhaps as a
maintenance manager operation), but that requires containers to have shared
ownership, a lifecycle change I'm not keen on making right now.

Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Reviewed-on: http://gerrit.cloudera.org:8080/6824
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
3 files changed, 157 insertions(+), 20 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I73a903092cf89508b7ba97fde3a2d6e691161b4c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] log block manager: convert container metrics from counters to gauges

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged.

Change subject: log block manager: convert container metrics from counters to 
gauges
..


log block manager: convert container metrics from counters to gauges

Container deletion is just around the corner, so these metrics need to
become gauges so that they can be decremented.

As an aside, it's possible that we've already been mishandling these
counters: they should never drop in value, even across restarts, and we were
incrementing them every time a container was reopened at startup. However,
we may have been OK because the block manager loads before the web UI, so by
the time the metrics are fetchable, the counter values should be the same as
what they were pre-restart.

Change-Id: I343d99d42da7f4fbc0facb476c97f2ca8785031d
Reviewed-on: http://gerrit.cloudera.org:8080/6823
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
2 files changed, 18 insertions(+), 26 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I343d99d42da7f4fbc0facb476c97f2ca8785031d
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] fs manager: optimize tmp file deletion

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: fs_manager: optimize tmp file deletion
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6837/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

PS1, Line 239: code:
 :   //
 :   //   Status s = fs_manager_->Open();
 :   //   if (s.IsNotFound()) {
 :   // 
RETURN_NOT_OK(fs_manager_->CreateInitialFileSystemLayout());
 :   // s = fs_manager_->Open();
 :   //   }
 :   //   RETURN_NOT_OK(s);
> I think instead of putting this code snippet here, you could just say "mini
Done


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

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


[kudu-CR] KUDU-1549: compact LBM container metadata files at startup

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: KUDU-1549: compact LBM container metadata files at startup
..


Patch Set 2:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/6826/2/src/kudu/fs/fs_report.h
File src/kudu/fs/fs_report.h:

Line 127: Entry(std::string c, BlockRecordPB* r);
> this is a little odd -- if it takes a pointer I expect it to store the poin
I'm pining away for protobuf move constructors.

(will add comment).


http://gerrit.cloudera.org:8080/#/c/6826/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

PS2, Line 87: DEFINE_double(log_container_live_metadata_before_compact_ratio, 
0.10,
:   "Desired ratio of live block metadata in log 
containers. If a "
:   "container's live to total block ratio dips below 
this value, "
:   "the container's metadata file will be compacted at 
startup.");
> should probably be tagged advanced or even experimental?
I'll tag it advanced. I don't think experimental is appropriate given that 
modifying it doesn't cause new code paths to execute; it just adjusts how often 
they run.

As for the default value...I don't have a strong opinion, but I'll note that 
the higher it is, the more metadata write amplification we suffer. That's why I 
was conservative to start. Did you have a particular value in mind?


Line 417:   BlockRecordPB* record,
> comment on the pointer semantics. it's not clear what's being "saved" here
Done


PS2, Line 1964: offsets also match
> how could the offsets of two live blocks match?
It's contrived, but the two blocks could be of zero length. I'll add a comment.


PS2, Line 1969: .swap(records);
> = std::move(records)? or use .emplace?
Done


Line 2258:   for (const auto& e : low_live_block_containers) {
> can you extract the body of this if to a new function, perhaps? or too many
Done


Line 2293:   continue;
> if this were extracted to a new function, this logic would be a bit easier 
Agreed, done.


http://gerrit.cloudera.org:8080/#/c/6826/2/src/kudu/fs/log_block_manager.h
File src/kudu/fs/log_block_manager.h:

PS2, Line 295:   // 3. Containers in 'low_live_block_containers' will have 
their metadata
 :   //files compacted.
 :  
> In a typical startup doesn't this mean a fairly large amount of memory usag
I went back and forth on this. I do like the idea of not making any on-disk 
changes until we've established that there are no fatal errors, and that's why 
I ended up on this side of the fence.

That said, here's an alternative: if you'll allow me to grow LogBlock by 8 
bytes, I can add the block creation timestamp to it, and then I can use the 
LogBlocks that we've already allocated in the compaction. There's another 
advantage to this approach: it opens the door to doing compaction at real time 
without the onerous burden of rereading records from disk.

Why didn't I do this already? I couldn't bring myself to grow our memory 
footprint by potentially dozens of MB without also implementing real time 
metadata compaction.


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

Line 60: #define KUDU_WARN_NOT_OK_AND_CONTINUE(to_call, warning_prefix) {   
\
> The naming here is a little confusing, considering 'continue' might be inte
I extracted a function like you suggested, so this has been removed.


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

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


[kudu-CR] KUDU-2005: actionable error messages from webserver

2017-05-10 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-2005: actionable error messages from webserver
..


Patch Set 1:

(1 comment)

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

PS1, Line 246: TryRunLsof(addr);
Put this after the error message below, since the below puts it in context.

It'd also be nice to do this lsof step only when we get an address-in-use 
error, or have good reason to suspect that is the error, rather than for any 
failure.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes


[kudu-CR] log: release WritableLogSegment buffers when log is idle

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: log: release WritableLogSegment buffers when log is idle
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6611/3/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:

PS3, Line 199: 1
flag?


http://gerrit.cloudera.org:8080/#/c/6611/3/src/kudu/consensus/log.h
File src/kudu/consensus/log.h:

Line 90: 
extra line


PS3, Line 99: Status SwitchToAsyncMode();
this new state change seems weird because it's one way. could we just refactor 
Append to just be wrapper around Status 
AsyncAppend(std::unique_ptr entry_batch,
 const StatusCallback& callback) and always have the log in 
async mode? do you think there would be a perf impact?


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

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


[kudu-CR] KUDU-2005: actionable error messages from webserver

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has uploaded a new change for review.

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

Change subject: KUDU-2005: actionable error messages from webserver
..

KUDU-2005: actionable error messages from webserver

As it turned out, squeasel outputs only errors via the log_message
callback.  Let's output them with ERROR severity into Kudu log
(they were output as INFO messages prior to this patch).

Also, if the embedded squeasel webserver fails to start, add the last
error message from it (if any) into the RuntimeError status message
returned from Webserver::Start().

Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
---
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
3 files changed, 25 insertions(+), 6 deletions(-)


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

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


[kudu-CR] log block manager: reopen container metadata writers at the OS level

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: log block manager: reopen container metadata writers at the OS 
level
..


Patch Set 2:

(and as long as you're writing the walls of text anyway might as well put them 
front and center, for posterity)

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

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


[kudu-CR] log block manager: reopen container metadata writers at the OS level

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: log block manager: reopen container metadata writers at the OS 
level
..


Patch Set 2:

gotta meet expectations

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

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


[kudu-CR] KUDU-1056 and KUDU-1020 Safe time for ksck checksum

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1056 and KUDU-1020 Safe time for ksck checksum
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib45be20dcfa37fb85185302adf84d2c4a55f8c1e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No


[kudu-CR] Fix flaky test TestRecoverFromOpIdOverflow

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has submitted this change and it was merged.

Change subject: Fix flaky test TestRecoverFromOpIdOverflow
..


Fix flaky test TestRecoverFromOpIdOverflow

This test is flaky because we race against the COMMIT message for the
first NO_OP in the WAL being written. It is currently hard to know when
the actual COMMIT message is written to the WAL so we use a workaround
to delete the first log segment before restarting the EMC in this test.

If the COMMIT doesn't get written in time then the tablet bootstrap
process doesn't prune that entry at startup time and it ends up in the
new log prior to the much higher-numbered log entries. The flaky test
failure was due to an out of order log index being detected, and looked
like the following error in the log:

F0504 21:28:21.128690 13908 raft_consensus_state.cc:502] Check failed: _s.ok() 
Bad status: Corruption: New operation's index does not follow the previous op's 
index. Current: 2147483648.2147483648. Previous: 1.1
*** Check failure stack trace: ***
@ 0x7fe0adef915d  google::LogMessage::Fail() at ??:0
@ 0x7fe0adefb05d  google::LogMessage::SendToLog() at ??:0
@ 0x7fe0adef8c99  google::LogMessage::Flush() at ??:0
@ 0x7fe0adefbaff  google::LogMessageFatal::~LogMessageFatal() at ??:0
@ 0x7fe0b57a8018  
kudu::consensus::PendingRounds::AdvanceCommittedIndex() at ??:0
@ 0x7fe0b57848a5  kudu::consensus::RaftConsensus::NotifyCommitIndex() 
at ??:0
@ 0x7fe0b5749ccf  
kudu::consensus::PeerMessageQueue::NotifyObserversOfCommitIndexChangeTask() at 
??:0
@ 0x7fe0b575bdc1  kudu::internal::RunnableAdapter<>::Run() at ??:0

Change-Id: Ib382819307da04bb76d68d2c015dc0edd9f60267
Reviewed-on: http://gerrit.cloudera.org:8080/6808
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/integration-tests/ts_recovery-itest.cc
1 file changed, 28 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib382819307da04bb76d68d2c015dc0edd9f60267
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 


[kudu-CR] log block manager: reopen container metadata writers at the OS level

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: log block manager: reopen container metadata writers at the OS 
level
..


Patch Set 1:

(1 comment)

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

PS1, Line 649: DCHECK_EQ(FileState::NOT_INITIALIZED, state_);
 :   RETURN_NOT_OK(ParsePBFileHeader(writer_.get(), &offset_, 
&version_));
 :   ContainerSupHeaderPB sup_header;
 :   RETURN_NOT_OK(ReadSupplementalHeader(writer_.get(), version_, 
&offset_, &sup_header));
 :   RETURN_NOT_OK(writer_->Size(&offset_)); // Reset the write 
offset to the end of the file.
 :   state_ = FileState::OPEN;
> nice writeup, could you copy paste this to the commit message?
Sure why not. JD told me me people already expect walls of text from me anyway 
and I hate to disappoint.


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

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


[kudu-CR] benchmarks: make ensure cpu scaling more resilient

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged.

Change subject: benchmarks: make ensure_cpu_scaling more resilient
..


benchmarks: make ensure_cpu_scaling more resilient

It's called out of a trap, and since the script uses pushd/popd a lot, it's
not always reachable:

  + python write-jobs-stats-to-mysql.py kudu-benchmarks 1039 
DenseNodeItest_time_bootstrapping_tablets 8
  usage: write-jobs-stats-to-mysql.py
 
  + restore_governor
  + ensure_cpu_scaling performance
  ++ dirname ./src/kudu/scripts/benchmarks.sh
  + ./src/kudu/scripts/ensure_cpu_scaling.sh performance
  ./src/kudu/scripts/benchmarks.sh: line 93: 
./src/kudu/scripts/ensure_cpu_scaling.sh: No such file or directory

Change-Id: I3452b48577dd8f5f72c57d41ccf41848c682d616
Reviewed-on: http://gerrit.cloudera.org:8080/6847
Tested-by: Adar Dembo 
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/scripts/benchmarks.sh
1 file changed, 2 insertions(+), 3 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3452b48577dd8f5f72c57d41ccf41848c682d616
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-1549: delete dead LBM containers at startup

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1549: delete dead LBM containers at startup
..


Patch Set 2: Code-Review+2

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

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


[kudu-CR] KUDU-1056 and KUDU-1020 Safe time for ksck checksum

2017-05-10 Thread Will Berkeley (Code Review)
Will Berkeley has posted comments on this change.

Change subject: KUDU-1056 and KUDU-1020 Safe time for ksck checksum
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6843/2/src/kudu/tools/ksck_remote-test.cc
File src/kudu/tools/ksck_remote-test.cc:

PS2, Line 154: a chance establish a timestamp.
> I see what you mean, my comment was more about verbage.
Ah ok. Your suggestion is much clearer so I will use something like it.

My comment about 1-2 tablets was misleading. More precisely, in the test table 
with 3 tablets, 9 replicas, I'd see 1-2 out of the 9 replicas have that 
problem-- not 2/3 replicas of the same tablet.


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

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


[kudu-CR] KUDU-1056 and KUDU-1020 Safe time for ksck checksum

2017-05-10 Thread Will Berkeley (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1056 and KUDU-1020 Safe time for ksck checksum
..

KUDU-1056 and KUDU-1020 Safe time for ksck checksum

Now that safe time works properly, this patch enables
the snapshot checksum tests for ksck.

Change-Id: Ib45be20dcfa37fb85185302adf84d2c4a55f8c1e
---
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck_remote-test.cc
2 files changed, 20 insertions(+), 28 deletions(-)


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

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


[kudu-CR] benchmarks: make ensure cpu scaling more resilient

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: benchmarks: make ensure_cpu_scaling more resilient
..


Patch Set 1: Code-Review+2

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

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


[kudu-CR] log block manager: reopen container metadata writers at the OS level

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: log block manager: reopen container metadata writers at the OS 
level
..


Patch Set 1:

(1 comment)

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

PS1, Line 649: DCHECK_EQ(FileState::NOT_INITIALIZED, state_);
 :   RETURN_NOT_OK(ParsePBFileHeader(writer_.get(), &offset_, 
&version_));
 :   ContainerSupHeaderPB sup_header;
 :   RETURN_NOT_OK(ReadSupplementalHeader(writer_.get(), version_, 
&offset_, &sup_header));
 :   RETURN_NOT_OK(writer_->Size(&offset_)); // Reset the write 
offset to the end of the file.
 :   state_ = FileState::OPEN;
> (I'm sure you're already aware of most of this long explanation, but I'm le
nice writeup, could you copy paste this to the commit message?


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

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


[kudu-CR] KUDU-1056 and KUDU-1020 Safe time for ksck checksum

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1056 and KUDU-1020 Safe time for ksck checksum
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6843/2/src/kudu/tools/ksck_remote-test.cc
File src/kudu/tools/ksck_remote-test.cc:

PS2, Line 154: a chance establish a timestamp.
> Without this change the test fails regularly with a message like:
I see what you mean, my comment was more about verbage.
Are you sure that you might get that number from 2 replicas, not just 1?
That lagging amount is interesting though, seems like the replica hasn't 
updated the safe time ever. Might actually be a bug.

Don't want to delay this though, so regarding verbage how about:
"Wait for the first 100 writes so be committed so that there is a very high 
chance that all replicas have committed at least one message in each tablet, 
otherwise safe time might not have been and replicas might refuse snapshot 
scans because of lag"

os something like that


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

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


[kudu-CR] log block manager: reopen container metadata writers at the OS level

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: log block manager: reopen container metadata writers at the OS 
level
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6825/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 2201: w.reset(new WritablePBContainerFile(std::move(f)));
> semi-unrelated question: now that we've had the file cache for a while and 
Sure, will do it in a follow-on patch.


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

PS2, Line 280: Init
> maybe best to rename this to OpenNew and the one below OpenForAppend or Ope
OK, I'll use CreateNew() and OpenExisting().


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

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


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

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has submitted this change and it was merged.

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


[catalog_manager] categorization of rw operation failures

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

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

If an operation against the system catalog fails within the same term
of catalog leadership, the error is considered fatal and that causes the
master process to crash (with an exception of writing a newly generated
TSK when the TokenSigner still has a TSK to use). This is to avoid a
possible inconsistency when working with the tables/tablets metadata,
the IPKI certificate authority information and the Token Signing Keys.

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

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

Added a test to verify that the master server does not crash if change
of leadership detected while trying to persist a newly generated TSK
(Token Signing Key).

Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Reviewed-on: http://gerrit.cloudera.org:8080/6170
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/catalog_manager_tsk-itest.cc
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/master_service.cc
M src/kudu/master/sys_catalog-test.cc
7 files changed, 506 insertions(+), 149 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 31
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] categorization of rw operation failures

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

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


Patch Set 30:

there you go :)

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

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


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

2017-05-10 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

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


Patch Set 30: Code-Review+2

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

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


[kudu-CR] KUDU-1952 Remove round-robin for block placement

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: KUDU-1952 Remove round-robin for block placement
..


Patch Set 18:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/6636/18/src/kudu/fs/block_manager-test.cc
File src/kudu/fs/block_manager-test.cc:

Line 144:   
Nit: got some extra whitespace here.


Line 325: CountFiles(path, &blocks_in_path);
Should ASSERT_OK() here.


http://gerrit.cloudera.org:8080/#/c/6636/17/src/kudu/fs/data_dirs-test.cc
File src/kudu/fs/data_dirs-test.cc:

Line 96:   }
> Done
Not done?


http://gerrit.cloudera.org:8080/#/c/6636/18/src/kudu/fs/data_dirs-test.cc
File src/kudu/fs/data_dirs-test.cc:

PS18, Line 47: Env::Default()
Use env_


Line 72:   testing::internal::Random r_;
This is a little unusual; use the Kudu Random from util/random.


http://gerrit.cloudera.org:8080/#/c/6636/17/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

Line 460: // natural tablet_id, select a data dir randomly.
> Done
Looks like you didn't add the DCHECK() though.


http://gerrit.cloudera.org:8080/#/c/6636/18/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

Line 21: #include 
Nit: this belongs with the gflags/glog includes since boost is part of the 
"project", not in the "system".


Line 540:   Random r(GetRandomSeed32());
Let's make this a member of the DataDirManager and initialize it just once, 
when constructing the DataDirManager.

Actually, let's make the output of std::default_random_engine() a member, and 
seed it with GetRandomSeed32().


http://gerrit.cloudera.org:8080/#/c/6636/17/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:

Line 51: 
> No, forward declaration usually doesn't work for STL containers since I'm n
Right, I forgot about that. Sorry.

Could you put the DataDirGroup into a namespace called 'internal' to make it 
clear that it's not for use outside of data_dirs.{h,cc}?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] WIP Allow external miniclusters to use many data dirs

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: WIP Allow external miniclusters to use many data dirs
..


Patch Set 1:

(1 comment)

I'll hold off on a full review until you're more comfortable with this and the 
compile/test failures have been addressed, but I did glance at the API.

http://gerrit.cloudera.org:8080/#/c/6845/1/src/kudu/integration-tests/external_mini_cluster.h
File src/kudu/integration-tests/external_mini_cluster.h:

Line 70:   // Number of directories to create for each daemon.
Wouldn't it be more natural to convert data_root into e.g. data_roots and allow 
multiple roots to be specified? That actually opens the door to running the 
mini cluster on a bunch of disks (provided the test code can collect the disk 
paths via gflag or some such).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2f5def6980ad394c8558ad97ba830f1b0257332
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] benchmarks: make ensure cpu scaling more resilient

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: benchmarks: make ensure_cpu_scaling more resilient
..


Patch Set 1: Verified+1

I tested this by running benchmarks.sh twice: once normally, and once I hit 
Ctrl-C _after_ a pushd. Both times ensure_cpu_scaling worked.

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

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


[kudu-CR] benchmarks: make ensure cpu scaling more resilient

2017-05-10 Thread Adar Dembo (Code Review)
Hello Todd Lipcon,

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

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

to review the following change.

Change subject: benchmarks: make ensure_cpu_scaling more resilient
..

benchmarks: make ensure_cpu_scaling more resilient

It's called out of a trap, and since the script uses pushd/popd a lot, it's
not always reachable:

  + python write-jobs-stats-to-mysql.py kudu-benchmarks 1039 
DenseNodeItest_time_bootstrapping_tablets 8
  usage: write-jobs-stats-to-mysql.py
 
  + restore_governor
  + ensure_cpu_scaling performance
  ++ dirname ./src/kudu/scripts/benchmarks.sh
  + ./src/kudu/scripts/ensure_cpu_scaling.sh performance
  ./src/kudu/scripts/benchmarks.sh: line 93: 
./src/kudu/scripts/ensure_cpu_scaling.sh: No such file or directory

Change-Id: I3452b48577dd8f5f72c57d41ccf41848c682d616
---
M src/kudu/scripts/benchmarks.sh
1 file changed, 2 insertions(+), 3 deletions(-)


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

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


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

2017-05-10 Thread Alexey Serbin (Code Review)
Alexey Serbin has posted comments on this change.

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


Patch Set 29:

(6 comments)

yep, it's been nits for the last 3 revisions :)

http://gerrit.cloudera.org:8080/#/c/6170/29/src/kudu/integration-tests/catalog_manager_tsk-itest.cc
File src/kudu/integration-tests/catalog_manager_tsk-itest.cc:

PS29, Line 163: cluster_->Shutdown();
> nit dont need this, the emc already shutsdown on the dctor
Done


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

PS29, Line 500:  might happen in case of a
> how about: can only happen in a
Done


PS29, Line 1026: PKI
> here and elsewhere, we tag our TODOs with usernames in most places
As I can see from the code, the better approach is to use TODO(KUDU-xxx) to 
have better tracking of issues.  For this particular case, KUDU-1920 is opened.


PS29, Line 4126: "uninitialized"
> this is redundant
Done


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

PS29, Line 360: // Calling this method makes sense only if leader_status.ok().
> How about: Requires: leader_status() returns OK()
Done


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

PS29, Line 202: that's a 
> remove
Done


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

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


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

2017-05-10 Thread Alexey Serbin (Code Review)
Hello David Ribeiro Alves, Adar Dembo,

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

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

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

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

[catalog_manager] categorization of rw operation failures

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

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

If an operation against the system catalog fails within the same term
of catalog leadership, the error is considered fatal and that causes the
master process to crash (with an exception of writing a newly generated
TSK when the TokenSigner still has a TSK to use). This is to avoid a
possible inconsistency when working with the tables/tablets metadata,
the IPKI certificate authority information and the Token Signing Keys.

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

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

Added a test to verify that the master server does not crash if change
of leadership detected while trying to persist a newly generated TSK
(Token Signing Key).

Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/catalog_manager_tsk-itest.cc
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/master_service.cc
M src/kudu/master/sys_catalog-test.cc
7 files changed, 506 insertions(+), 149 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/6170/30
-- 
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: 30
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6846/1/java/kudu-client/pom.xml
File java/kudu-client/pom.xml:

Line 144
To remove this, I think you need to add attachProtoSources=false to the 
protobuf-maven-plugin configuration. Please test by extracting the kudu-client 
JAR and looking for .proto files inside (there should be none).


Line 138: 
com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
If you don't mind please try this out on SLES 12 SP1, Red Hat or CentOS 6.6, 
Ubuntu 14.04, and Debian 8. You don't actually need to run through the Java 
build (unless you want to); IIUC there will be one executable for any x86_64 
Linux, so just find it and try to run it on those platforms. If it fails to run 
because of a missing glibc symbol version, we've got problems.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 1:

(1 comment)

I suspect the test failures may be due to endianess issues in the byte buffers.

http://gerrit.cloudera.org:8080/#/c/6846/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java:

Line 30: import com.google.protobuf.UnsafeByteOperations;
duplicate


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Grant Henke has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
..


Patch Set 1:

Note: Currently compiling the proto files in this build will dump warnings 
about the syntax field missing. I will upgrade the proto version in the rest of 
the codebase and add syntax=proto2 in a follow up patch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] KUDU-1988: add support for advertised host:port info.

2017-05-10 Thread Patrik Sundberg (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1988: add support for advertised host:port info.
..

KUDU-1988: add support for advertised host:port info.

Change-Id: I6735ca5630fc4c426bf72d0b21d6ef452173a890
---
M src/kudu/master/master.cc
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/webserver.cc
M src/kudu/server/webserver.h
M src/kudu/server/webserver_options.cc
M src/kudu/server/webserver_options.h
M src/kudu/tserver/heartbeater.cc
8 files changed, 68 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6735ca5630fc4c426bf72d0b21d6ef452173a890
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Patrik Sundberg 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Patrik Sundberg 
Gerrit-Reviewer: Tidy Bot


[kudu-CR] [java-client] Update protoc and simplify the maven build

2017-05-10 Thread Grant Henke (Code Review)
Grant Henke has uploaded a new change for review.

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

Change subject: [java-client] Update protoc and simplify the maven build
..

[java-client] Update protoc and simplify the maven build

This patch simplifies the Java build by using maven to
provide protoc via the Maven OS plugin and the
protocArtifact property of the Maven Protobuf plugin.

It also updates protoc to 3.3.0 and makes the required
changes due to compatibility breaks.

Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
---
M docs/installation.adoc
M java/README.md
M java/kudu-client/pom.xml
D 
java/kudu-client/src/main/java/com/google/protobuf/ZeroCopyLiteralByteString.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/ColumnRangePredicate.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestColumnRangePredicate.java
M java/pom.xml
15 files changed, 60 insertions(+), 252 deletions(-)


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

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


[kudu-CR] KUDU-1988: add support for advertised host:port info.

2017-05-10 Thread Patrik Sundberg (Code Review)
Patrik Sundberg has posted comments on this change.

Change subject: KUDU-1988: add support for advertised host:port info.
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6827/4/src/kudu/server/rpc_server.h
File src/kudu/server/rpc_server.h:

Line 70:   Status GetAdvertisedAddresses(std::vector* addresses) 
const WARN_UNUSED_RESULT;
> Oh interesting, if it's normal for the advertised address to be globally re
I'm not aware of a case where it wouldn't be intended to be resolvable on both 
sides of the private/public split that's causing us to want to use an 
advertised address. And for cases when you want to use an advertised address, 
your deployment would be such that you can know at least the hostname which 
will get the public IP at config time, so hence that's what you use in config 
as the advertised name. At runtime this will be guaranteed to resolve, as it'd 
be done before container is running.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6735ca5630fc4c426bf72d0b21d6ef452173a890
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Patrik Sundberg 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Patrik Sundberg 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes


[kudu-CR] KUDU-1988: add support for advertised host:port info.

2017-05-10 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: KUDU-1988: add support for advertised host:port info.
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6827/4/src/kudu/server/rpc_server.h
File src/kudu/server/rpc_server.h:

Line 70:   Status GetAdvertisedAddresses(std::vector* addresses) 
const WARN_UNUSED_RESULT;
> I have not checked yet, was meant to today but got sidelined. My thinking w
Oh interesting, if it's normal for the advertised address to be globally 
resolvable, then this way is fine.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6735ca5630fc4c426bf72d0b21d6ef452173a890
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Patrik Sundberg 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Patrik Sundberg 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes


  1   2   >