[kudu-CR] docs: improvements to transaction semantics

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9235 )

Change subject: docs: improvements to transaction semantics
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc
File docs/transaction_semantics.adoc:

http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc@59
PS1, Line 59: with different levels of consistency or correctness guarantees. 
Scans can perform time-travel reads, i.e.
> nit: is this over the col limit? (seems like it, but i might be wrong as I
Done


http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc@181
PS1, Line 181: `READ_LATEST`:: This is the default read mode. The server takes 
a snapshot of
 : the MVCC state and proceeds with the read immediately. Reads in 
this mode only yield
 : 'Read Committed' isolation.
> are you leaving the new mode out on purpose?
Yeah, I am planning to do another patch to add it. This patch is only updating 
existing ones.


http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc@205
PS1, Line 205: Note
> don't we have a special "note" marker, or am I mistaken?
Done


http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc@222
PS1, Line 222: Writes
> ahah, well caught :)
:)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23a2751923a4214f52e6cdb233f7e3aeee207da2
Gerrit-Change-Number: 9235
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 21 Feb 2018 07:43:33 +
Gerrit-HasComments: Yes

[kudu-CR] docs: improvements to transaction semantics

[Hao Hao (Code Review)]

Hello Alex Rodoni, David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: docs: improvements to transaction semantics
..

docs: improvements to transaction semantics

Change-Id: I23a2751923a4214f52e6cdb233f7e3aeee207da2
---
M docs/transaction_semantics.adoc
1 file changed, 21 insertions(+), 8 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23a2751923a4214f52e6cdb233f7e3aeee207da2
Gerrit-Change-Number: 9235
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode

[Hao Hao (Code Review)]

Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
..

KUDU-1704: add java client support for READ_YOUR_WRITES mode

Change-Id: I6239521c022147257859e399f55c6f3f945af465
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
4 files changed, 196 insertions(+), 4 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack 
tracing
..

KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing

Previously we used glog's wrapper around libunwind for stack tracing.
However that has a deficiency that it assumes that, process wide, only
one thread can be inside libunwind at a time[1]

It appears that this is left over from some very old versions of
libunwind, or was already unnecessarily conservative. libunwind is meant
to be thread safe, and we have tests that will trigger if it is not.

This just extracts the function body of the glog function we were using
and does the same work manually.

Without this fix, the "collect from all the threads at the same time"
code path resulted in most of the threads collecting an empty trace
since they tried to call libunwind at the same time.

[1] https://github.com/google/glog/issues/298
Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634
---
M build-support/iwyu/iwyu.sh
A build-support/iwyu/mappings/libunwind.imp
M src/kudu/util/debug-util-test.cc
M src/kudu/util/debug-util.cc
M src/kudu/util/debug-util.h
5 files changed, 81 insertions(+), 17 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634
Gerrit-Change-Number: 9319
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2291 (part 5): allow collecting stack traces asynchronously

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2291 (part 5): allow collecting stack traces asynchronously
..

KUDU-2291 (part 5): allow collecting stack traces asynchronously

This removes the global variable previously used to communicate between
a stack trace collector and the stack trace target. The global variable
had the downside of allowing only a single stack trace operation in
flight at a time, which means that for use cases where we want to
capture the trace of a bunch of threads, we needed to do so serially.

For use cases like process-wide traces this could make debugging a bit
tricky. For example, because of the skew in time between collecting
the stack of thread A and the stack of thread B, it's possible that
the traces could show them holding the same lock, which we know to be
impossible. Getting the collection time as close as possible to
"simultaneous" can reduce the possibility of these kind of
strange-looking results.

The actual mechanism to do this is slightly tricky. Rather than
duplicate the full description here, check the comments in the
implementation. In particular, StackTraceCollector::RevokeSigData()
explains the complexity of not knowing whether our signal will ever be
delivered.

A new unit test shows the pattern, and this also updates the stack
servlet to send the signals to all of the threads as close as possible
to "the same time".

Change-Id: Ie4d7789bb1272db033df4685f46eaf8483ec7be1
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/util/debug-util-test.cc
M src/kudu/util/debug-util.cc
M src/kudu/util/debug-util.h
4 files changed, 317 insertions(+), 109 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie4d7789bb1272db033df4685f46eaf8483ec7be1
Gerrit-Change-Number: 9318
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2297 (part 4): periodically dump stacks to diagnostics log

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9330 )

Change subject: KUDU-2297 (part 4): periodically dump stacks to diagnostics log
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9330/5/src/kudu/server/diagnostics_log.cc
File src/kudu/server/diagnostics_log.cc:

http://gerrit.cloudera.org:8080/#/c/9330/5/src/kudu/server/diagnostics_log.cc@27
PS5, Line 27: #include 
> warning: #includes are not sorted properly [llvm-include-order]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic32abf2c48ac6a5f3c384e58838b34671bcaf147
Gerrit-Change-Number: 9330
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 07:14:45 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2291 (part 5): allow collecting stack traces asynchronously

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9318 )

Change subject: KUDU-2291 (part 5): allow collecting stack traces asynchronously
..


Patch Set 4:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.h
File src/kudu/util/debug-util.h:

http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.h@208
PS4, Line 208: Trigger
> TriggerAsync
Done


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc
File src/kudu/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@141
PS4, Line 141:   void Signal() {
> add: // Wakes all waiters, notifying them of completion.
Done


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@151
PS4, Line 151:   bool WaitUntil(MonoTime deadline) {
> add: // Returns false if reached deadline before completion, true otherwise
Done


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@216
PS4, Line 216: maybe
> nit: capitalize
Done


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@219
PS4, Line 219: ANNOTATE_HAPPENS_AFTER(sig);
> why is this needed?
Because we're using the signal data as a sort of queue to move the 'info' 
pointer between two threads, we need this to ensure that TSAN sees there is an 
enforced ordering between the two. Normally we get this ordering implied when 
thread A acquires a mutex or spinlock previously released by thread B, but in 
this case there is no such mutex.


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@230
PS4, Line 230: 2
> nit: /*skip_frames=*/
Done


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@312
PS4, Line 312: }
> nit: move up one line
Done


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@395
PS4, Line 395: SYS_rt_tgsigqueueinfo
> Did you look into using pthread_sigqueue() ? I suppose a minor benefit of t
yea, the problem is that for pthread_sigqueue we need to have pthread_t for all 
the threads, and best I can tell there is no way to go from a tid to a 
pthread_t. Additionally in the past I looked into how to list all running 
threads using pthreads APIs and came up with nothing particularly nice. There's 
some 'thread_db' API but it seems meant for debuggers and not easy to use at 
all.


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@396
PS4, Line 396: )
> , ErrnoToString(errno), errno) ?
this ends up printed in the /stacks page and stuff, so I think it's better to 
just have the nice message here.


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@427
PS4, Line 427: Status StackTraceCollector::TriggerAsync(int64_t tid_, 
StackTrace* stack) {
> missing void StackTraceCollector::RevokeSigData() for non-linux
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4d7789bb1272db033df4685f46eaf8483ec7be1
Gerrit-Change-Number: 9318
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 07:13:38 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2291 (part 4): avoid potential for deadlocks in stack trace collection

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9262 )

Change subject: KUDU-2291 (part 4): avoid potential for deadlocks in stack 
trace collection
..


Patch Set 6: Code-Review+2

(1 comment)

Carrying forward +2

http://gerrit.cloudera.org:8080/#/c/9262/5/src/kudu/util/debug/unwind_safeness.cc
File src/kudu/util/debug/unwind_safeness.cc:

http://gerrit.cloudera.org:8080/#/c/9262/5/src/kudu/util/debug/unwind_safeness.cc@23
PS5, Line 23: // Based on public domain code by Aliaksey Kandratsenka at
> Do we need to add this to LICENSE?
It's only super loosely based on it, so I don't think so (eg his original was 
ANSI C). Plus public domain has no reporting requirements by ASF policy AFAIK



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I414718a26c4810de86a10397bb9803dc5e386d1a
Gerrit-Change-Number: 9262
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 06:08:00 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2291 (part 4): avoid potential for deadlocks in stack trace collection

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9262 )

Change subject: KUDU-2291 (part 4): avoid potential for deadlocks in stack 
trace collection
..

KUDU-2291 (part 4): avoid potential for deadlocks in stack trace collection

This follows some recommendations from [1] to avoid the potential for
deadlocks if we attempt to gather a stack trace from a signal handler
context while the thread is inside calls to libdl.

The approach is somewhat ugly: we have to override the sensitive libdl
calls and use them to increment a thread-local counter indicating that
it would be unsafe to collect a stack. We then check this counter before
capturing a stack: if we see that we are inside libdl we just return an
fake stack trace indicating why it could not be captured.

The new test included deadlocks reliably without the fix.

[1] 
https://github.com/gperftools/gperftools/wiki/gperftools'-stacktrace-capturing-methods-and-their-issues
Change-Id: I414718a26c4810de86a10397bb9803dc5e386d1a
Reviewed-on: http://gerrit.cloudera.org:8080/9262
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/util/CMakeLists.txt
M src/kudu/util/debug-util-test.cc
M src/kudu/util/debug-util.cc
M src/kudu/util/debug-util.h
A src/kudu/util/debug/unwind_safeness.cc
A src/kudu/util/debug/unwind_safeness.h
6 files changed, 291 insertions(+), 10 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I414718a26c4810de86a10397bb9803dc5e386d1a
Gerrit-Change-Number: 9262
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2291 (part 3): use futex to speed up stack collection

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9254 )

Change subject: KUDU-2291 (part 3): use futex to speed up stack collection
..

KUDU-2291 (part 3): use futex to speed up stack collection

Previously the thread which requests a stack trace would poll on an
atomic variable waiting for the stack to be collected. Of course
spinning is too CPU-hungry so it would actually sleep for 10ms in
each iteration. This limited the performance of stack collection to
about 100/second, which is problematic for the /stacks web page on a
server that may have thousands of threads.

This switches to using the futex system call to allow the dumper thread
to sleep and the dumpee thread to wake it back up when the results are
ready.

Why do we use the low level futex and not something like a condition
variable or CountdownLatch? The issue is that the dumpee thread is
running in a signal handler context, and most things are not safe to do
in that context. Particularly, mutexes may block, which is very naughty,
and condition variables and latches are currently implemented on top of
mutexes. pthread semaphores are another option, but those don't support
proper timeout semantics, which is a feature we use in this use case.

Whether the futex syscall itself is async-signal-safe is up for some debate.
Clearly the "wait" mode is not, since it may block, but is the "wake" mode
that we use here allowed? Technically I don't think so. In practice,
the pthread sem_post() API _is_ documented to be async-signal-safe and
I verified that in libc source it's implemented using futex to wake the
waiter. So, if we are doing something illegal, at least we're doing
the same illegal thing as libc, so we're very unlikely to ever break.

Obviously futex is not portable to macOS, but this code was already
Linux-specific so we haven't lost much.

This speeds up the benchmark for stack trace collection (without symbolization)
by 585x:

Before:
  I0207 19:33:20.960477 30799 debug-util-test.cc:175] Throughput: 99 
dumps/second (symbolize=0)
  I0207 19:33:21.963946 30799 debug-util-test.cc:175] Throughput: 84 
dumps/second (symbolize=1)

After:
  I0207 19:38:32.505213 32039 debug-util-test.cc:175] Throughput: 57916 
dumps/second (symbolize=0)
  I0207 19:38:33.506417 32039 debug-util-test.cc:175] Throughput: 595 
dumps/second (symbolize=1)

Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
Reviewed-on: http://gerrit.cloudera.org:8080/9254
Reviewed-by: Mike Percy 
Tested-by: Kudu Jenkins
---
M src/kudu/util/debug-util.cc
1 file changed, 70 insertions(+), 16 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
Gerrit-Change-Number: 9254
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-1704: add c++ client support for READ YOUR WRITES mode

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 10:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client-internal.h@197
PS9, Line 197:
 :   //
> this sentence is non-sensical. also max/latest is bound to be confusing. re
Makes sense. Updated.


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client-internal.h@201
PS9, Line 201:   //errors, timeouts, or leadership issues.
 :   // 3) 'deadline' (if initialized) elapses.
 :   //
 :   // NOTE: 'rpc_timeout' is a per-call timeout, while
> also non-sensical; also see my comment above
Done


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

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client-test.cc@992
PS9, Line 992:  ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 27, 
kNoBound));
 :   ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 0, 
15));
 :   ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 0, 
10));
 :   ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 0, 
20));
 :   ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 0, 
30));
 :   ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 14, 
30));
 :   ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 30, 
30));
 :   ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 
kTabletsNum * kRowsPerTablet,
 :kNoBound));
 : }
 : INSTANTIATE_TEST_CASE_P(Params, ScanMultiTabletParamTest,
 : testing::ValuesIn(read_modes));
 :
 : TEST_F(ClientTest, TestScanEmptyTable) {
 :   K
> can you make this a parameterized test that received the read mode as input
Done


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h@1740
PS9, Line 1740: /// When @c READ_YOUR_WRITES is specified, the server will 
perform a server-local
> I still think there is room to simplify these docs.  I would prefer if they
Done


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h@1743
PS9, Line 1743: minimize
> nit: verbage (... ensures ... minimize sounds weird)
Done


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h@1745
PS9, Line 1745:
> What does this mean in terms of an application developer reading this doc?
Done


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/scan_token-internal.cc@253
PS9, Line 253: LOG(FATAL) << "Ignoring snapshot timestamp since not in "
 :   "READ_AT_SNAPSH
> should we validate this on the config? i.e. don't allow to pass a snapshot
Done


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/scanner-internal.cc@330
PS9, Line 330:   if (configuration_.has_snapshot_timestamp()) {
 : LOG(FATAL) << "Ignoring snapshot timestamp since "
 :   "not in READ_AT_SNAPSHOT mode.";
 :   }
> same comment I did elsewhere
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 21 Feb 2018 05:24:02 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1704: add c++ client support for READ YOUR WRITES mode

[Hao Hao (Code Review)]

Hello Alex Rodoni, Mike Percy, Dan Burkert, David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..

KUDU-1704: add c++ client support for READ_YOUR_WRITES mode

Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_configuration.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
10 files changed, 302 insertions(+), 61 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

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

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

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


Patch Set 18:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163:
> Thanks for the explanation. Let's add that explanation in the comment here,
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163:
> Maybe we should just add that explanation to the docs?
Updated.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 05:23:20 +
Gerrit-HasComments: Yes

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

[Hao Hao (Code Review)]

Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

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

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

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

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

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 326 insertions(+), 98 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-721: Support range partitions on decimal columns

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9363 )

Change subject: KUDU-721: Support range partitions on decimal columns
..


Patch Set 2:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9363/1/src/kudu/client/client-test.cc@1639
PS1, Line 1639: schema;
> nit: I don't think we use identifiers like 'x_' for local variables; would
Ah, right. I copied this test from the UnixTimeMicros one above. Will update 
both.


http://gerrit.cloudera.org:8080/#/c/9363/1/src/kudu/client/client-test.cc@1643
PS1, Line 1643: ASSERT_O
> why not just ASSERT_OK() ?
Done


http://gerrit.cloudera.org:8080/#/c/9363/1/src/kudu/client/client-test.cc@1645
PS1, Line 1645:   unique_ptr lower_bound(schema.NewRow());
  :   ASSERT_OK(lower_bound->SetUnscaledDecimal("key", -1));
  :   unique_ptr upper_bound(schema.NewRow());
  :   ASSERT_OK(upper_bound->SetUnscaledDecimal("key", 99));
> I think it would be nice to add a 'negative' test case (maybe, in some diff
Done


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

http://gerrit.cloudera.org:8080/#/c/9363/1/src/kudu/common/partition.cc@1043
PS1, Line 1043: const ColumnS
> const reference?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I419d442dd233c73166c6e391907c8443ebecc917
Gerrit-Change-Number: 9363
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 21 Feb 2018 05:01:38 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-721: Support range partitions on decimal columns

[Grant Henke (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: KUDU-721: Support range partitions on decimal columns
..

KUDU-721: Support range partitions on decimal columns

Add the DataType handling to enable range partitions on
decimal columns and a test that fails without the changes.

Also adds range partition tests for edge cases where:
* The upper bound is less than than the lower bound
* The upper bound is equal to the lower bound
* The bounds use MIN/MAX values

Change-Id: I419d442dd233c73166c6e391907c8443ebecc917
---
M src/kudu/client/client-test.cc
M src/kudu/common/partition.cc
M src/kudu/util/decimal_util-test.cc
M src/kudu/util/decimal_util.cc
M src/kudu/util/decimal_util.h
5 files changed, 155 insertions(+), 9 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I419d442dd233c73166c6e391907c8443ebecc917
Gerrit-Change-Number: 9363
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2291 (part 3): use futex to speed up stack collection

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9254 )

Change subject: KUDU-2291 (part 3): use futex to speed up stack collection
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
Gerrit-Change-Number: 9254
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 03:07:21 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2291 (part 5): allow collecting stack traces asynchronously

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9318 )

Change subject: KUDU-2291 (part 5): allow collecting stack traces asynchronously
..


Patch Set 5:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.h
File src/kudu/util/debug-util.h:

http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.h@208
PS4, Line 208: Trigger
TriggerAsync


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc
File src/kudu/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@141
PS4, Line 141:   void Signal() {
add: // Wakes all waiters, notifying them of completion.


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@151
PS4, Line 151:   bool WaitUntil(MonoTime deadline) {
add: // Returns false if reached deadline before completion, true otherwise.


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@216
PS4, Line 216: maybe
nit: capitalize


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@219
PS4, Line 219: ANNOTATE_HAPPENS_AFTER(sig);
why is this needed?


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@230
PS4, Line 230: 2
nit: /*skip_frames=*/


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@312
PS4, Line 312: }
nit: move up one line


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@395
PS4, Line 395: SYS_rt_tgsigqueueinfo
Did you look into using pthread_sigqueue() ? I suppose a minor benefit of this 
approach is that we can use the native tid?


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@396
PS4, Line 396: )
, ErrnoToString(errno), errno) ?


http://gerrit.cloudera.org:8080/#/c/9318/4/src/kudu/util/debug-util.cc@427
PS4, Line 427: Status StackTraceCollector::TriggerAsync(int64_t tid_, 
StackTrace* stack) {
missing void StackTraceCollector::RevokeSigData() for non-linux


http://gerrit.cloudera.org:8080/#/c/9318/5/src/kudu/util/debug-util.cc
File src/kudu/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/9318/5/src/kudu/util/debug-util.cc@213
PS5, Line 213: sig
nit: name this sig_data for consistency when tracing both sides of the 
communication logic.


http://gerrit.cloudera.org:8080/#/c/9318/5/src/kudu/util/debug-util.cc@350
PS5, Line 350: DLOG(WARNING) << "Leaking SignalData structure " << 
sig_data_ << " after lost signal "
I think we could at least make the signalled thread try to delete the sig_data 
structure if the caller has given up. I haven't tested this but I think we 
could use something like:

  -  if (!sig->queued_to_tid.compare_exchange_strong(my_tid, 
SignalData::kDumpInProgress)) {
  +  int64_t old_val = sig->queued_to_tid.exchange(SignalData::kDumpInProgress);
  +  if (old_val != my_tid) {
  +delete sig;
   return;
 }



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4d7789bb1272db033df4685f46eaf8483ec7be1
Gerrit-Change-Number: 9318
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 03:06:10 +
Gerrit-HasComments: Yes

[kudu-CR] WIP: KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9375


Change subject: WIP: KUDU-2297 (part 5): dump stacks into diagnostics log on 
service queue overflow
..

WIP: KUDU-2297 (part 5): dump stacks into diagnostics log on service queue 
overflow

In the past we've had several bugs and performance issues which manifest
themselves as service queue overflows. It would be easier to pinpoint
the root cause (eg slow IO vs lock contention) if we had a process-wide
stack snapshot at the time of the overflow.

This patch does some plumbing to trigger such a stack trace into the
diagnostics log when a service queue overflow occurs. The logging is
throttled to once every 5 seconds so that we don't contribute too much
to the load itself.

The plumbing is a bit ugly since the diagnostics log is in the server/
module whereas the overflows surface in the rpc/ module. Having rpc/
call back into server/ would be a cyclic dependency, so instead the rpc/
module exposes a std::function<> style callback for the server to hook
into.

WIP because I'd like to add some kind of test for this, though I tested
manually and it appears to work.

Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
---
M src/kudu/rpc/service_pool.cc
M src/kudu/rpc/service_pool.h
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
M src/kudu/server/rpc_server.cc
M src/kudu/server/rpc_server.h
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
8 files changed, 96 insertions(+), 11 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8
Gerrit-Change-Number: 9375
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase
..

KUDU-2297 (part 1): refactor metrics log out of ServerBase

This is preparing for putting more information into the metrics log
instead of just metrics snapshots. The logging code will get more
complex as it gains features, so this makes a new class for it.

This is slightly incompatible since I also changed the name on disk to
'diagnostics' instead of 'metrics' and updated the documentation, but I
am not aware of people using this in the wild. So long as we
release-note it, I think it's reasonable to expect any sophisticated
operators to adjust their scripting accordingly.

Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d
---
M docs/administration.adoc
M src/kudu/server/CMakeLists.txt
A src/kudu/server/diagnostics_log.cc
A src/kudu/server/diagnostics_log.h
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
6 files changed, 241 insertions(+), 73 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d
Gerrit-Change-Number: 9326
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2291 (part 3): use futex to speed up stack collection

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2291 (part 3): use futex to speed up stack collection
..

KUDU-2291 (part 3): use futex to speed up stack collection

Previously the thread which requests a stack trace would poll on an
atomic variable waiting for the stack to be collected. Of course
spinning is too CPU-hungry so it would actually sleep for 10ms in
each iteration. This limited the performance of stack collection to
about 100/second, which is problematic for the /stacks web page on a
server that may have thousands of threads.

This switches to using the futex system call to allow the dumper thread
to sleep and the dumpee thread to wake it back up when the results are
ready.

Why do we use the low level futex and not something like a condition
variable or CountdownLatch? The issue is that the dumpee thread is
running in a signal handler context, and most things are not safe to do
in that context. Particularly, mutexes may block, which is very naughty,
and condition variables and latches are currently implemented on top of
mutexes. pthread semaphores are another option, but those don't support
proper timeout semantics, which is a feature we use in this use case.

Whether the futex syscall itself is async-signal-safe is up for some debate.
Clearly the "wait" mode is not, since it may block, but is the "wake" mode
that we use here allowed? Technically I don't think so. In practice,
the pthread sem_post() API _is_ documented to be async-signal-safe and
I verified that in libc source it's implemented using futex to wake the
waiter. So, if we are doing something illegal, at least we're doing
the same illegal thing as libc, so we're very unlikely to ever break.

Obviously futex is not portable to macOS, but this code was already
Linux-specific so we haven't lost much.

This speeds up the benchmark for stack trace collection (without symbolization)
by 585x:

Before:
  I0207 19:33:20.960477 30799 debug-util-test.cc:175] Throughput: 99 
dumps/second (symbolize=0)
  I0207 19:33:21.963946 30799 debug-util-test.cc:175] Throughput: 84 
dumps/second (symbolize=1)

After:
  I0207 19:38:32.505213 32039 debug-util-test.cc:175] Throughput: 57916 
dumps/second (symbolize=0)
  I0207 19:38:33.506417 32039 debug-util-test.cc:175] Throughput: 595 
dumps/second (symbolize=1)

Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
---
M src/kudu/util/debug-util.cc
1 file changed, 70 insertions(+), 16 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
Gerrit-Change-Number: 9254
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2297 (part 4): periodically dump stacks to diagnostics log

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2297 (part 4): periodically dump stacks to diagnostics log
..

KUDU-2297 (part 4): periodically dump stacks to diagnostics log

This modifies the diagnostics log to periodically dump stack traces.
This is slightly complicated by the fact that symbolized stack traces
can be relatively large. So, we separate the logging of symbols and
stack traces. When an address first appears in a log file, it is logged
as part of a symbol line. Later logs of the same address do not need
to re-log the symbol.

With this, a typical stack trace dump of an idle tserver is about 4KB
pre-compression, and a 'symbols' dump is about 6KB. So logging stacks
reasonably often should not use much disk space or IO.

Currently this is enabled on the same interval as the metrics log, but
only if a new experimental flag --diagnostics-log-stack-traces is
enabled. I'm planning to move it to a different flag in a later commit,
but wanted to keep this one simple and incremental.

Change-Id: Ic32abf2c48ac6a5f3c384e58838b34671bcaf147
---
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
M src/kudu/util/debug-util.cc
M src/kudu/util/debug-util.h
M src/kudu/util/rolling_log-test.cc
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
7 files changed, 227 insertions(+), 41 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic32abf2c48ac6a5f3c384e58838b34671bcaf147
Gerrit-Change-Number: 9330
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [catalog manager] check re-replication scheme upon table creation

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9321 )

Change subject: [catalog_manager] check re-replication scheme upon table 
creation
..

[catalog_manager] check re-replication scheme upon table creation

Output actionable warning message upon creation of a new table when
the number of live tablet servers is not enough to re-replicate
tablet replicas in case of a failure.

Since the 3-4-3 replication scheme is now enabled by default, this might
be useful in case if running a cluster with just N tablet servers when
newly created tables have replication factor of N
(e.g., imagine running with just 3 tablet servers total and creating
tables with the default replication factor of 3).

This is a follow-up for c40e0587bf6a6aa55e5bd72dd2dd9356b1507f2e.

Change-Id: I5a59451ad876d9864a90607187a05f8526cd8cbf
Reviewed-on: http://gerrit.cloudera.org:8080/9321
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/master/catalog_manager.cc
1 file changed, 36 insertions(+), 16 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5a59451ad876d9864a90607187a05f8526cd8cbf
Gerrit-Change-Number: 9321
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] WIP KUDU-2319 follower masters cannot verify tokens

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP KUDU-2319 follower masters cannot verify tokens
..

WIP KUDU-2319 follower masters cannot verify tokens

This small test explains that follower masters still cannot accept
authn tokens for verification: they don't have the public parts of
TSKs generated by the leader master.

WIP: it's necessary to fix the issue -- make follower masters to obtain
 the public parts of currently avaialable TSK keys for TokenVerifier

WIP: after fixing the issue, it's necessary to enable currently disabled
 FollowerMastersHaveTokenVerificationKeys test.

Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/integration-tests/security-master-certificates-itest.cc
2 files changed, 76 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Gerrit-Change-Number: 9373
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2274 [itest] stress test for replica replacement

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9255 )

Change subject: KUDU-2274 [itest] stress test for replica replacement
..

KUDU-2274 [itest] stress test for replica replacement

This is a stress test for the automatic replica replacement in Kudu.

Parameters of the test are configurable via run-time gflags, so it's
possible to run it in a 'standalone' mode, targeting it to be a sort
of an endurance test.

This test reproduces the race described in KUDU-2274: it triggers
DCHECK() assertions added in 194fd8b169f29aafbd78a47709ac51d2e8354a1a
before the relevant fixes for KUDU-2274 were checked in.

Change-Id: I036a882f7e9132a5c26013227c50c0699b59ed6e
Reviewed-on: http://gerrit.cloudera.org:8080/9255
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/integration-tests/cluster_verifier.h
A src/kudu/integration-tests/raft_consensus_stress-itest.cc
4 files changed, 305 insertions(+), 4 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I036a882f7e9132a5c26013227c50c0699b59ed6e
Gerrit-Change-Number: 9255
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-2259: add real user to AuthenticationCredentialsPB

[Dan Burkert (Code Review)]

Hello Todd Lipcon,

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

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

to review the following change.


Change subject: KUDU-2259: add real user to AuthenticationCredentialsPB
..

KUDU-2259: add real user to AuthenticationCredentialsPB

WIP: the Java client needs a similar change

This commit adds the 'real user' to the authn credentials token, which
is used when negotiating connections with SASL PLAIN authentication.
This is useful when scan tokens are being sent to remote tasks, it's not
possible to authenticate with a signed authn token to the remote
server[1], coarse-grained ACLs have been set, and the 'planner' and
'executor' processes are being run with different users.

[1]: this most often occurs because the remote server has encryption disabled.

Change-Id: I5d2d901d42501ecfc0f6372f68cf7335eb188b45
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.proto
M src/kudu/client/master_rpc.cc
M src/kudu/client/master_rpc.h
M src/kudu/client/meta_cache.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/rpc/connection_id.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/user_credentials.cc
M src/kudu/rpc/user_credentials.h
M src/kudu/util/user.cc
14 files changed, 102 insertions(+), 10 deletions(-)



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

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

[kudu-CR] WIP follower masters cannot verify tokens

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9373


Change subject: WIP follower masters cannot verify tokens
..

WIP follower masters cannot verify tokens

This small test explains that follower masters still cannot accept
authn tokens for verification: they don't have the public parts of
TSKs generated by the leader master.

WIP: it's necessary to fix the issue -- make follower masters to obtain
 the public parts of currently avaialable TSK keys for TokenVerifier

Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/integration-tests/security-master-certificates-itest.cc
2 files changed, 64 insertions(+), 0 deletions(-)



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

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

[kudu-CR] gutil: properly hook up ANNOTATE HAPPENS BEFORE/AFTER

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9325 )

Change subject: gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
..

gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER

This updates dynamic_annotations to hook up ANNOTATE_HAPPENS_BEFORE and
ANNOTATE_HAPPENS_AFTER to the appropriate annotations in the TSAN
runtime library. According to [1] (committed in 2011) the functions have
been implemented in TSAN for quite some time, and it was an error that
we weren't using them.

The previous implementation of the function called some
condition-variable annotations in the TSAN runtime library instead.
Those functions actually turn out to be implemented as no-ops. So, I
looked for existing call sites to ANNOTATE_HAPPENS_BEFORE and AFTER and
removed them. This cleaned up atomic_refcount pretty substantially.
Again I found a matching change[2] in Chromium, from which this code is
derived.

[1] https://codereview.chromium.org/6982022
[2] https://codereview.chromium.org/580813002

Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
Reviewed-on: http://gerrit.cloudera.org:8080/9325
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/gutil/atomic_refcount.h
M src/kudu/gutil/dynamic_annotations.c
M src/kudu/gutil/dynamic_annotations.h
M src/kudu/gutil/once.cc
M src/kudu/gutil/once.h
5 files changed, 22 insertions(+), 59 deletions(-)

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

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

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

[kudu-CR] KUDU-2274 [itest] stress test for replica replacement

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9255 )

Change subject: KUDU-2274 [itest] stress test for replica replacement
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I036a882f7e9132a5c26013227c50c0699b59ed6e
Gerrit-Change-Number: 9255
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 01:37:02 +
Gerrit-HasComments: No

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

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

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


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163: (propagation ts + 1) should not
> Maybe we should just add that explanation to the docs?
Thanks for the explanation. Let's add that explanation in the comment here, 
maybe something like:

  // fall into that category because calling 
clock->Update(propagated_timestamp) verifies that propagated_timestamp + 
FLAGS_max_clock_sync_error_usec is not too far into the future.



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

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

[kudu-CR] gutil: properly hook up ANNOTATE HAPPENS BEFORE/AFTER

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9325 )

Change subject: gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
Gerrit-Change-Number: 9325
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 01:22:02 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2291 (part 4): avoid potential for deadlocks in stack trace collection

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9262 )

Change subject: KUDU-2291 (part 4): avoid potential for deadlocks in stack 
trace collection
..


Patch Set 5: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9262/5/src/kudu/util/debug/unwind_safeness.cc
File src/kudu/util/debug/unwind_safeness.cc:

http://gerrit.cloudera.org:8080/#/c/9262/5/src/kudu/util/debug/unwind_safeness.cc@23
PS5, Line 23: // Based on public domain code by Aliaksey Kandratsenka at
Do we need to add this to LICENSE?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I414718a26c4810de86a10397bb9803dc5e386d1a
Gerrit-Change-Number: 9262
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 01:17:58 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2291 (part 4): avoid potential for deadlocks in stack trace collection

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9262 )

Change subject: KUDU-2291 (part 4): avoid potential for deadlocks in stack 
trace collection
..


Patch Set 5:

(2 comments)

Nice. How the heck did you even stumble upon this workaround?

http://gerrit.cloudera.org:8080/#/c/9262/5/src/kudu/util/debug/unwind_safeness.cc
File src/kudu/util/debug/unwind_safeness.cc:

http://gerrit.cloudera.org:8080/#/c/9262/5/src/kudu/util/debug/unwind_safeness.cc@53
PS5, Line 53: libdl
nit: period at end


http://gerrit.cloudera.org:8080/#/c/9262/5/src/kudu/util/debug/unwind_safeness.cc@63
PS5, Line 63: *
nit: any reason to move the asterisks around from their usual place in these 
signatures?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I414718a26c4810de86a10397bb9803dc5e386d1a
Gerrit-Change-Number: 9262
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 01:12:49 +
Gerrit-HasComments: Yes

[kudu-CR] rowset metadata: cache min/max encoded keys

[Andrew Wong (Code Review)]

Andrew Wong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9372


Change subject: rowset_metadata: cache min/max encoded keys
..

rowset_metadata: cache min/max encoded keys

This patch adds a new flag rowset_metadata_store_keys that, when true,
will indicate that Kudu should duplicate diskrowset min/max keys into
the rowset metadata. Doing so allows Kudu to read the keys from tablet
metadata and bootstrap tablets without having to fully open the
CFileReaders for the key columns of each rowset.

A small test is added to tablet_server-test that ensures we don't read
any extraneous bytes when starting up the tablet server if we're reading
keys from the rowset metadata.

Change-Id: I37d6f7160e3a7188753684e063963110f70e9b8d
---
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/rowset_metadata.cc
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tserver/tablet_server-test.cc
7 files changed, 121 insertions(+), 17 deletions(-)



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

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

[kudu-CR] docs: improvements to transaction semantics

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9235 )

Change subject: docs: improvements to transaction semantics
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc
File docs/transaction_semantics.adoc:

http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc@59
PS1, Line 59: with different levels of consistency or correctness guarantees. 
Scans can perform time-travel reads, i.e.
nit: is this over the col limit? (seems like it, but i might be wrong as I 
forget if we have one for docs or what it is)


http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc@181
PS1, Line 181: `READ_LATEST`:: This is the default read mode. The server takes 
a snapshot of
 : the MVCC state and proceeds with the read immediately. Reads in 
this mode only yield
 : 'Read Committed' isolation.
are you leaving the new mode out on purpose?


http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc@205
PS1, Line 205: Note
don't we have a special "note" marker, or am I mistaken?


http://gerrit.cloudera.org:8080/#/c/9235/1/docs/transaction_semantics.adoc@222
PS1, Line 222: Writes
ahah, well caught :)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23a2751923a4214f52e6cdb233f7e3aeee207da2
Gerrit-Change-Number: 9235
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 21 Feb 2018 01:00:06 +
Gerrit-HasComments: Yes

[kudu-CR] WIP [tests] scenario to repro off-by-one error in TestWorkload

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9277 )

Change subject: WIP [tests] scenario to repro off-by-one error in TestWorkload
..


Patch Set 1:

I briefly tried this with my WIP implementation of leader leases and the 
problem seemed to have gone away. While not definitive this is encouraging 
evidence that the problem indeed stems from the lack of leader leases.
Conceptually this seems plausible too. Scenario:
- Two replicas (a,b) dispute leadership of tablet A, i.e. both think they're 
leaders, but "b" is the actual most recent leader.
- Client writes the last row to replica "b".
- Client then performs a scan at snapshot for all rows on replica "a", sending 
the timestamp of the write.
- Replica "a" thinking it's leader simply considers the current timestamp 
"safe" and executes a (non-repeatable) scan that doesn't include the last row 
written to b.

Possible follow-up steps:
- Retry the scan. If the problem goes away, then it's more likely that the 
problem stems from the lack of leader leases.
- Even better, hack it so that we retry the scan _at the same snapshot 
timestamp_.
- Add logging events (test only) that register the timestamps/safe time/chosen 
replica on the server side (hopefully showing the smoking gun).
- Implement leader leader and put this test on top :)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60666b8b05dce8dd13fcdee6408c0930915ba0c1
Gerrit-Change-Number: 9277
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 21 Feb 2018 00:54:04 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2291 (part 3): use futex to speed up stack collection

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9254 )

Change subject: KUDU-2291 (part 3): use futex to speed up stack collection
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9254/5/src/kudu/util/debug-util.cc
File src/kudu/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/9254/5/src/kudu/util/debug-util.cc@144
PS5, Line 144: 0
> how about: nullptr /* ignored */
Done


http://gerrit.cloudera.org:8080/#/c/9254/5/src/kudu/util/debug-util.cc@197
PS5, Line 197: spins waiting
> s/spins waiting/waits/
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
Gerrit-Change-Number: 9254
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 00:49:59 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8847 )

Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode
..


Patch Set 4:

I'm slacking on this review a bit until we have something solid on the c++ side 
(which we seem close to now). it's easier to review by analogy :)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465
Gerrit-Change-Number: 8847
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 21 Feb 2018 00:44:02 +
Gerrit-HasComments: No

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

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

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


Patch Set 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163: (propagation ts + 1) should not
> As we update the clock based on propagated timestamp at L2140, if (propagat
Maybe we should just add that explanation to the docs?
Also: obvious while we have these things in context, but likely worth 
mentioning for the documentation value is that "clean" timestamp is, by 
definition in the past.
Suggestion, something like:
There is no need to validate if the chosen timestamp is too far in the future, 
as :
- MVCC's 'clean' ts is by definition in the past (it's maximally bounded by 
safe time).
- "propagated" ts was used to update the clock above and the update would have 
returned an error if the the timestamp was too far in the future.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 21 Feb 2018 00:42:13 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2291 (part 3): use futex to speed up stack collection

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9254 )

Change subject: KUDU-2291 (part 3): use futex to speed up stack collection
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9254/5/src/kudu/util/debug-util.cc
File src/kudu/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/9254/5/src/kudu/util/debug-util.cc@144
PS5, Line 144: 0
how about: nullptr /* ignored */


http://gerrit.cloudera.org:8080/#/c/9254/5/src/kudu/util/debug-util.cc@197
PS5, Line 197: spins waiting
s/spins waiting/waits/



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb772f0607275ec340c7f38be05347fc14820cbd
Gerrit-Change-Number: 9254
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 00:37:42 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2297 (part 4): periodically dump stacks to diagnostics log

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2297 (part 4): periodically dump stacks to diagnostics log
..

KUDU-2297 (part 4): periodically dump stacks to diagnostics log

This modifies the diagnostics log to periodically dump stack traces.
This is slightly complicated by the fact that symbolized stack traces
can be relatively large. So, we separate the logging of symbols and
stack traces. When an address first appears in a log file, it is logged
as part of a symbol line. Later logs of the same address do not need
to re-log the symbol.

With this, a typical stack trace dump of an idle tserver is about 4KB
pre-compression, and a 'symbols' dump is about 6KB. So logging stacks
reasonably often should not use much disk space or IO.

Currently this is enabled on the same interval as the metrics log, but
only if a new experimental flag --diagnostics-log-stack-traces is
enabled. I'm planning to move it to a different flag in a later commit,
but wanted to keep this one simple and incremental.

Change-Id: Ic32abf2c48ac6a5f3c384e58838b34671bcaf147
---
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
M src/kudu/util/debug-util.cc
M src/kudu/util/debug-util.h
M src/kudu/util/rolling_log-test.cc
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
7 files changed, 227 insertions(+), 41 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic32abf2c48ac6a5f3c384e58838b34671bcaf147
Gerrit-Change-Number: 9330
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack 
tracing
..

KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing

Previously we used glog's wrapper around libunwind for stack tracing.
However that has a deficiency that it assumes that, process wide, only
one thread can be inside libunwind at a time[1]

It appears that this is left over from some very old versions of
libunwind, or was already unnecessarily conservative. libunwind is meant
to be thread safe, and we have tests that will trigger if it is not.

This just extracts the function body of the glog function we were using
and does the same work manually.

Without this fix, the "collect from all the threads at the same time"
code path resulted in most of the threads collecting an empty trace
since they tried to call libunwind at the same time.

[1] https://github.com/google/glog/issues/298
Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634
---
M build-support/iwyu/iwyu.sh
A build-support/iwyu/mappings/libunwind.imp
M src/kudu/util/debug-util-test.cc
M src/kudu/util/debug-util.cc
M src/kudu/util/debug-util.h
5 files changed, 81 insertions(+), 17 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634
Gerrit-Change-Number: 9319
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2297 (part 3): refactor process-wide stack collection out of /stacks

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2297 (part 3): refactor process-wide stack collection out 
of /stacks
..

KUDU-2297 (part 3): refactor process-wide stack collection out of /stacks

Previously a bunch of logic to collect all the stacks from the process
was in the /stacks path handler. This logic is relatively generic and
shouldn't be intermingled with the formatting code. In particular I'd
like to use it in the diagnostics log, where a different output format
is desirable.

Change-Id: Ibb7c6edd31254f3d7e0cbef1eaf575bde3570df6
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/diagnostics_log.cc
M src/kudu/util/debug-util-test.cc
M src/kudu/util/debug-util.cc
M src/kudu/util/debug-util.h
5 files changed, 190 insertions(+), 112 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb7c6edd31254f3d7e0cbef1eaf575bde3570df6
Gerrit-Change-Number: 9329
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2297 (part 2). Convert diagnostics log to a format closer to glog

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2297 (part 2). Convert diagnostics log to a format closer 
to glog
..

KUDU-2297 (part 2). Convert diagnostics log to a format closer to glog

Previously the metrics/diagnostics log used a custom log format designed
to be readable by scripts. The only timestamps were unix microtimes,
which are convenient for computers but not for people. In order to make
the log more easily greppable, this patch converts it over to use a
glog-compatible prefix on each log line.

The aim is that, if an admin sees some issues at 10/23 15:03 they can simply
grep the diagnostics log for '1021 15:0[0-6]' rather than having to go
figure out the appropriate range of numeric timestamps.

This patch also updates the docs accordingly.

Change-Id: I10d93c84f8c738c26850ae3d1ae6afbb5519c7ad
---
M docs/administration.adoc
M src/kudu/server/diagnostics_log.cc
M src/kudu/server/diagnostics_log.h
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/trace.cc
6 files changed, 47 insertions(+), 29 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I10d93c84f8c738c26850ae3d1ae6afbb5519c7ad
Gerrit-Change-Number: 9327
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

[Todd Lipcon (Code Review)]

Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,

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

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

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

Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase
..

KUDU-2297 (part 1): refactor metrics log out of ServerBase

This is preparing for putting more information into the metrics log
instead of just metrics snapshots. The logging code will get more
complex as it gains features, so this makes a new class for it.

This is slightly incompatible since I also changed the name on disk to
'diagnostics' instead of 'metrics' and updated the documentation, but I
am not aware of people using this in the wild. So long as we
release-note it, I think it's reasonable to expect any sophisticated
operators to adjust their scripting accordingly.

Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d
---
M docs/administration.adoc
M src/kudu/server/CMakeLists.txt
A src/kudu/server/diagnostics_log.cc
A src/kudu/server/diagnostics_log.h
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
6 files changed, 240 insertions(+), 73 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d
Gerrit-Change-Number: 9326
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] internal mini cluster: support Cluster/LogVerifier

[Andrew Wong (Code Review)]

Andrew Wong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9137 )

Change subject: internal_mini_cluster: support Cluster/LogVerifier
..

internal_mini_cluster: support Cluster/LogVerifier

This patch introduces a MiniCluster-agnostic MiniClusterFsInspector.
With this, the LogVerifier, and thus, the ClusterVerifier can support
both External- and InternalMiniClusters.

The LogVerifier would originally open a read-only FsManager (which in
turn would open a BlockManager and a DataDirManager) per server and pass
it to the LogReader to inspect the WALs. Instead, the LogVerifier now
creates a MiniClusterFsInspector, which is more lightweight and defined
per cluster. To the LogReader, it passes the WAL directory and an env,
which is all the LogReader needed from the FsManager in the first place.

To test, I updated a test case in ts_tablet_manager-itest to make use of
the new ClusterVerifier with an internal cluster. CheckCluster() uses a
LogVerifier, which in this case uses an MiniClusterFsInspector that
operates on an InternalMiniCluster.

Change-Id: I228a6e3ba1a42db4e243ffdc5116f0c60ee04a84
Reviewed-on: http://gerrit.cloudera.org:8080/9137
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_reader.h
M src/kudu/fs/fs_manager.h
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/external_mini_cluster-itest-base.cc
M src/kudu/integration-tests/external_mini_cluster-itest-base.h
M src/kudu/integration-tests/log_verifier.cc
M src/kudu/integration-tests/log_verifier.h
R src/kudu/integration-tests/mini_cluster_fs_inspector.cc
R src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/raft_config_change-itest.cc
M src/kudu/integration-tests/raft_consensus-itest-base.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/raft_consensus_election-itest.cc
M src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/tablet_copy_client_session-itest.cc
M src/kudu/integration-tests/tablet_replacement-itest.cc
M src/kudu/integration-tests/tombstoned_voting-itest.cc
M src/kudu/integration-tests/tombstoned_voting-stress-test.cc
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/kudu-ts-cli-test.cc
37 files changed, 313 insertions(+), 305 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I228a6e3ba1a42db4e243ffdc5116f0c60ee04a84
Gerrit-Change-Number: 9137
Gerrit-PatchSet: 13
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2291 (part 2): Add a /stacks page

[Mike Percy (Code Review)]

Mike Percy has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9253 )

Change subject: KUDU-2291 (part 2): Add a /stacks page
..

KUDU-2291 (part 2): Add a /stacks page

This adds a simple /stacks web page which dumps the currently running
threads in a plain text format.

Since we often have a lot of idle threadpool threads sitting around in
the "wait for work" state, the output collapses all threads with
matching stacks and only displays the stack once, making it more
suitable for human consumption.

Example output from a local kudu-master:
  https://gist.github.com/b64739ee5fb146ea1953380f57b996c4

Longer term we may want to integrate stack-tracing capability into the
/threadz view as well, but for now I left this as a low-level utility
which doesn't access the thread manager, etc. I left a few TODOs for
further enhancements, but I've already found this helpful for
understanding some perf anomalies while playing with YCSB, so let's get
it committed and improve as we go.

Change-Id: I8b8f6d50d44e40fd51357fdbfd8f9ba2ebaa724b
Reviewed-on: http://gerrit.cloudera.org:8080/9253
Tested-by: Todd Lipcon 
Reviewed-by: Mike Percy 
---
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/server/default_path_handlers.cc
M src/kudu/util/debug-util.cc
3 files changed, 106 insertions(+), 9 deletions(-)

Approvals:
  Todd Lipcon: Verified
  Mike Percy: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8b8f6d50d44e40fd51357fdbfd8f9ba2ebaa724b
Gerrit-Change-Number: 9253
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2291 (part 2): Add a /stacks page

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9253 )

Change subject: KUDU-2291 (part 2): Add a /stacks page
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f6d50d44e40fd51357fdbfd8f9ba2ebaa724b
Gerrit-Change-Number: 9253
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Wed, 21 Feb 2018 00:07:14 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9343 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..


Patch Set 9:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9343/8/src/kudu/rpc/rpc-test.cc@28
PS8, Line 28: #include 
> warning: #includes are not sorted properly [llvm-include-order]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:56:34 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Tidy Bot, Lars Volker, Alexey Serbin, Dan Burkert, Kudu 
Jenkins, Todd Lipcon,

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

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

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

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..

KUDU-2301: (Part-1) Add instrumentation on a per connection level

This patch returns the OutboundTransfer queue size on a per
connection level and makes them accessible via the
DumpRunningRpcs() call.

A test is added in rpc-test to ensure that this metric works as
expected.

A future patch will add more metrics.

Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/connection.h
M src/kudu/rpc/messenger.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_introspection.proto
5 files changed, 68 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 9
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2274 [itest] stress test for replica replacement

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9255 )

Change subject: KUDU-2274 [itest] stress test for replica replacement
..


Patch Set 6:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9255/5//COMMIT_MSG@11
PS5, Line 11: ar
> are
Done


http://gerrit.cloudera.org:8080/#/c/9255/5/src/kudu/integration-tests/raft_consensus_stress-itest.cc
File src/kudu/integration-tests/raft_consensus_stress-itest.cc:

http://gerrit.cloudera.org:8080/#/c/9255/5/src/kudu/integration-tests/raft_consensus_stress-itest.cc@236
PS5, Line 236:
> nit: why not iteration++ here?
Because I don't want to count retry due to a ksck failure as an iteration: if 
RunKsck() fails, the logic starts another loop, but  that case is not counted 
as 'iteration' in sense of this scenario.


http://gerrit.cloudera.org:8080/#/c/9255/5/src/kudu/integration-tests/raft_consensus_stress-itest.cc@277
PS5, Line 277:   if (ksck_failures_in_a_row > FLAGS_test_max_ksck_failures) {
 : // Suspecting a Raft consensus failure while running ksck 
with shorter
 : // timeout (see above). Run an extra round of ksck with the 
regular timeout
 : // to verify that replicas haven't really converged and, if 
so, just bail
 : // right at this point.
 : const auto& s = v.RunKsck();
 : if (!s.ok()) {
 :   FAIL() << Substitute("$0: tablet replicas haven't 
converged", s.ToString());
 : }
 :   }
> isn't this the same as line 288 where we run v.CheckCluster() ?
Yes, CheckCluster contains RunKsck().  However, it's wrapped into some extra 
logic for retries.  Here I wanted to short-circuit and report an error right 
away if the ksck has already failed too many times in a row.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I036a882f7e9132a5c26013227c50c0699b59ed6e
Gerrit-Change-Number: 9255
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:54:32 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2274 [itest] stress test for replica replacement

[Alexey Serbin (Code Review)]

Hello Tidy Bot, Mike Percy,

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

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

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

Change subject: KUDU-2274 [itest] stress test for replica replacement
..

KUDU-2274 [itest] stress test for replica replacement

This is a stress test for the automatic replica replacement in Kudu.

Parameters of the test are configurable via run-time gflags, so it's
possible to run it in a 'standalone' mode, targeting it to be a sort
of an endurance test.

This test reproduces the race described in KUDU-2274: it triggers
DCHECK() assertions added in 194fd8b169f29aafbd78a47709ac51d2e8354a1a
before the relevant fixes for KUDU-2274 were checked in.

Change-Id: I036a882f7e9132a5c26013227c50c0699b59ed6e
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/integration-tests/cluster_verifier.h
A src/kudu/integration-tests/raft_consensus_stress-itest.cc
4 files changed, 305 insertions(+), 4 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I036a882f7e9132a5c26013227c50c0699b59ed6e
Gerrit-Change-Number: 9255
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

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

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

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


Patch Set 16:

(10 comments)

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

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc@a1726
PS16, Line 1726:
> What happened to this test?  Maybe I'm missing something, but it seems this
Since ScannerOpenWhenServerShutsDownParamTest should also cover the scenario of 
this test case. I removed it as it is duplicated.


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@157
PS16, Line 157: TimeStamp
> nit: in the code around, we have Timestamp as a type; maybe, name it consis
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@161
PS16, Line 161: PickAndVerifyTimeStamp
> nit: ditto, PickAndVerifyTimestamp(...) ?
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@162
PS16, Line 162: tablet::TabletReplica*
> Could it be const tablet::TabletReplica& ?
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@1757
PS16, Line 1757:   case READ_YOUR_WRITES:
> nit: per coding standard, add // fallthrough
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2042
PS16, Line 2042: case READ_AT_SNAPSHOT:
> nit: per coding standard, add // fallthrough
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2117
PS16, Line 2117: .
> s/./,/
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2118
PS16, Line 2118: initiated
> s/initiated with/default-constructed to/
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2118
PS16, Line 2118: Since
> s/Since/since/
Done


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163: (propagation ts + 1) should not
> is that guaranteed by something?
As we update the clock based on propagated timestamp at L2140, if (propagation 
ts + 1) is a timestamp too far in the future, which essentially means 
(propagation TS + 1 > propagation TS + FLAGS_max_clock_sync_error_usec).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:50:48 +
Gerrit-HasComments: Yes

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

[Hao Hao (Code Review)]

Hello Tidy Bot, Alexey Serbin, Mike Percy, Dan Burkert, David Ribeiro Alves, 
Kudu Jenkins, Andrew Wong,

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

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

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

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

KUDU-1704: add READ_YOUR_WRITES scan mode

This patch adds a new read mode READ_YOUR_WRITES on tserver. In this mode,
the server will pick a snapshot in the past, subject to the propagated
timestamp bound, and perform a read. Moreover, the chosen snapshot
timestamp is returned back to the client.

The major difference between READ_AT_SNAPSHOT scan without a timestamp
and READ_YOUR_WRITES scan is the latter will choose the newest timestamp
within the staleness bound that allows execution of the reads without
being blocked by the in-flight transactions if possible. READ_YOUR_WRITES
mode is not repeatable. However, it allows client local
read-your-writes/read-your-reads.

Design doc: 
https://docs.google.com/document/d/1WRLzKdCmRxXjUpi-DZsz7l2gl6215BsNdORtDXrmgl0/edit?usp=sharing

Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
---
M src/kudu/common/common.proto
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
6 files changed, 322 insertions(+), 98 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 17
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9343 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..


Patch Set 8:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9343/7/src/kudu/rpc/rpc-test.cc@494
PS7, Line 494:   ASSERT_OK(client_messenger->DumpRunningRpcs(dump_req, 
_resp));
> Can you try looping this test with --stress-cpu-threads=$(nproc) to double
I ran the test with --stress_cpu_threads=$(nrpoc) for about an hour (~800 
iterations), and there were no failures.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 8
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:44:39 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9326 )

Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.h
File src/kudu/server/diagnostics_log.h:

http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.h@39
PS3, Line 39:   DiagnosticsLog(std::string log_dir,
> warning: function 'kudu::server::DiagnosticsLog::DiagnosticsLog' has a defi
Done


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

http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc@104
PS3, Line 104: Unable to collect metrics to diagnostics log
> Add a bit saying we'll retry in `kWaitBetweenFailures` seconds.
Done


http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc@118
PS3, Line 118: this seems redundant with the "only include changed metrics" 
above
> Referring to `opts.only_modified_in_or_after_epoch` below?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d
Gerrit-Change-Number: 9326
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:44:07 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Lars Volker, Alexey Serbin, Dan Burkert, Kudu Jenkins, Todd 
Lipcon,

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

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

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

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..

KUDU-2301: (Part-1) Add instrumentation on a per connection level

This patch returns the OutboundTransfer queue size on a per
connection level and makes them accessible via the
DumpRunningRpcs() call.

A test is added in rpc-test to ensure that this metric works as
expected.

A future patch will add more metrics.

Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/connection.h
M src/kudu/rpc/messenger.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_introspection.proto
5 files changed, 68 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 8
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2274 [itest] stress test for replica replacement

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9255 )

Change subject: KUDU-2274 [itest] stress test for replica replacement
..


Patch Set 5: Code-Review+1

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9255/5//COMMIT_MSG@11
PS5, Line 11: is
are


http://gerrit.cloudera.org:8080/#/c/9255/5/src/kudu/integration-tests/raft_consensus_stress-itest.cc
File src/kudu/integration-tests/raft_consensus_stress-itest.cc:

http://gerrit.cloudera.org:8080/#/c/9255/5/src/kudu/integration-tests/raft_consensus_stress-itest.cc@236
PS5, Line 236:
nit: why not iteration++ here?


http://gerrit.cloudera.org:8080/#/c/9255/5/src/kudu/integration-tests/raft_consensus_stress-itest.cc@277
PS5, Line 277:   if (ksck_failures_in_a_row > FLAGS_test_max_ksck_failures) {
 : // Suspecting a Raft consensus failure while running ksck 
with shorter
 : // timeout (see above). Run an extra round of ksck with the 
regular timeout
 : // to verify that replicas haven't really converged and, if 
so, just bail
 : // right at this point.
 : const auto& s = v.RunKsck();
 : if (!s.ok()) {
 :   FAIL() << Substitute("$0: tablet replicas haven't 
converged", s.ToString());
 : }
 :   }
isn't this the same as line 288 where we run v.CheckCluster() ?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I036a882f7e9132a5c26013227c50c0699b59ed6e
Gerrit-Change-Number: 9255
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:42:10 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] check re-replication scheme upon table creation

[Alexey Serbin (Code Review)]

Hello Mike Percy, Kudu Jenkins,

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

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

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

Change subject: [catalog_manager] check re-replication scheme upon table 
creation
..

[catalog_manager] check re-replication scheme upon table creation

Output actionable warning message upon creation of a new table when
the number of live tablet servers is not enough to re-replicate
tablet replicas in case of a failure.

Since the 3-4-3 replication scheme is now enabled by default, this might
be useful in case if running a cluster with just N tablet servers when
newly created tables have replication factor of N
(e.g., imagine running with just 3 tablet servers total and creating
tables with the default replication factor of 3).

This is a follow-up for c40e0587bf6a6aa55e5bd72dd2dd9356b1507f2e.

Change-Id: I5a59451ad876d9864a90607187a05f8526cd8cbf
---
M src/kudu/master/catalog_manager.cc
1 file changed, 36 insertions(+), 16 deletions(-)


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

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

[kudu-CR] [catalog manager] check re-replication scheme upon table creation

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9321 )

Change subject: [catalog_manager] check re-replication scheme upon table 
creation
..


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9321/1/src/kudu/master/catalog_manager.cc@1386
PS1, Line 1386: live
> nit: s/alive/live/
Done


http://gerrit.cloudera.org:8080/#/c/9321/1/src/kudu/master/catalog_manager.cc@1401
PS1, Line 1401:
> add: (not recommended)
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a59451ad876d9864a90607187a05f8526cd8cbf
Gerrit-Change-Number: 9321
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:13:11 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] check re-replication scheme upon table creation

[Alexey Serbin (Code Review)]

Hello Mike Percy,

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

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

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

Change subject: [catalog_manager] check re-replication scheme upon table 
creation
..

[catalog_manager] check re-replication scheme upon table creation

Output actionable warning message upon creation of a new table when
the number of alive tablet servers is not enough to re-replicate
tablet replicas in case of a failure.

Since the 3-4-3 replication scheme is now enabled by default, this might
be useful in case if running a cluster with just N tablet servers when
newly created tables have replication factor of N
(e.g., imagine running with just 3 tablet servers total and creating
tables with the default replication factor of 3).

This is a follow-up for c40e0587bf6a6aa55e5bd72dd2dd9356b1507f2e.

Change-Id: I5a59451ad876d9864a90607187a05f8526cd8cbf
---
M src/kudu/master/catalog_manager.cc
1 file changed, 36 insertions(+), 16 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5a59451ad876d9864a90607187a05f8526cd8cbf
Gerrit-Change-Number: 9321
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-2305: Fix local variable usage to handle 2GB messages

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9355 )

Change subject: KUDU-2305: Fix local variable usage to handle 2GB messages
..


Patch Set 3: Code-Review+1

(1 comment)

Would be good for Dan B to take a look at this as our resident int overflow 
detective

http://gerrit.cloudera.org:8080/#/c/9355/3/src/kudu/rpc/serialization.cc
File src/kudu/rpc/serialization.cc:

http://gerrit.cloudera.org:8080/#/c/9355/3/src/kudu/rpc/serialization.cc@61
PS3, Line 61: static_cast(additional_size)
> Yes, I think only the first cast is necessary to fix the ASAN issue. This s
could go either way I guess. I often get confused by type promotion rules like 
this so I'm not against being explicit.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e468099b16f7eb55de71b753acae8f57d18287c
Gerrit-Change-Number: 9355
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:10:25 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] check re-replication scheme upon table creation

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9321 )

Change subject: [catalog_manager] check re-replication scheme upon table 
creation
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9321/1/src/kudu/master/catalog_manager.cc@1386
PS1, Line 1386: alive
nit: s/alive/live/


http://gerrit.cloudera.org:8080/#/c/9321/1/src/kudu/master/catalog_manager.cc@1401
PS1, Line 1401: .
add: (not recommended)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a59451ad876d9864a90607187a05f8526cd8cbf
Gerrit-Change-Number: 9321
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:08:21 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-721: Support range partitions on decimal columns

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9363 )

Change subject: KUDU-721: Support range partitions on decimal columns
..


Patch Set 1:

LGTM with Alexey's points addressed


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I419d442dd233c73166c6e391907c8443ebecc917
Gerrit-Change-Number: 9363
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 20 Feb 2018 23:07:43 +
Gerrit-HasComments: No

[kudu-CR] KUDU-721: Support range partitions on decimal columns

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9363 )

Change subject: KUDU-721: Support range partitions on decimal columns
..


Patch Set 1:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9363/1/src/kudu/client/client-test.cc@1639
PS1, Line 1639: u_schema_
nit: I don't think we use identifiers like 'x_' for local variables; would 
'u_schema' suffice here?


http://gerrit.cloudera.org:8080/#/c/9363/1/src/kudu/client/client-test.cc@1643
PS1, Line 1643: CHECK_OK
why not just ASSERT_OK() ?


http://gerrit.cloudera.org:8080/#/c/9363/1/src/kudu/client/client-test.cc@1645
PS1, Line 1645:   unique_ptr lower_bound(u_schema_.NewRow());
  :   ASSERT_OK(lower_bound->SetUnscaledDecimal("key", -1));
  :   unique_ptr upper_bound(u_schema_.NewRow());
  :   ASSERT_OK(upper_bound->SetUnscaledDecimal("key", 99));
I think it would be nice to add a 'negative' test case (maybe, in some 
different scenario) to test for:
  * swapped upper/lower bounds (e.g., upper -1, lower 99)
  * bounds that lead to an empty partition (e.g. lower 10, upper 10)
  * bounds with MAX and MIN values


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

http://gerrit.cloudera.org:8080/#/c/9363/1/src/kudu/common/partition.cc@1043
PS1, Line 1043: ColumnSchema
const reference?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I419d442dd233c73166c6e391907c8443ebecc917
Gerrit-Change-Number: 9363
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:59:18 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9343 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..


Patch Set 7:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9343/7/src/kudu/rpc/rpc-test.cc@494
PS7, Line 494:   
ASSERT_GT(dump_resp.outbound_connections(0).outbound_queue_size(), 0);
Can you try looping this test with --stress-cpu-threads=$(nproc) to double 
check it isn't flaky?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 7
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:38:19 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9343 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9343/5/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/9343/5/src/kudu/rpc/rpc-test.cc@489
PS5, Line 489:
> I still don't think that helps, because the RPCs will still end up read off
Ah right, that was a mistake.

I changed it to keep the server's reactor thread busy by adding a sleep task 
for 2 seconds before queuing up RPCs.

I tried using a latch, but the reactor thread restrictions hits a DCHECK, as 
we're not supposed to block on reactor threads.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 7
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:36:30 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Lars Volker, Alexey Serbin, Dan Burkert, Kudu Jenkins, Todd 
Lipcon,

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

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

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

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..

KUDU-2301: (Part-1) Add instrumentation on a per connection level

This patch returns the OutboundTransfer queue size on a per
connection level and makes them accessible via the
DumpRunningRpcs() call.

A test is added in rpc-test to ensure that this metric works as
expected.

A future patch will add more metrics.

Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/connection.h
M src/kudu/rpc/messenger.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_introspection.proto
5 files changed, 66 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 7
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [TestWorkload] an option to retry on read timeouts

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9295 )

Change subject: [TestWorkload] an option to retry on read timeouts
..


Patch Set 4:

Thanks you Todd and Mike for the reviews.  Yes, I agree it makes sense to 
verify if that was an exposure of some issues on the client side.  I'll take a 
look at that a bit later, maybe in the scope of wrapping up along with 
https://gerrit.cloudera.org/c/7037/


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54a6b81e4ba3a55676f1fc97cd577a5fe545c550
Gerrit-Change-Number: 9295
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:29:09 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2297 (part 1): refactor metrics log out of ServerBase

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9326 )

Change subject: KUDU-2297 (part 1): refactor metrics log out of ServerBase
..


Patch Set 3:

(2 comments)

SGTM except nits. Release failure looks unrelated. The TSAN failure may be 
legit, though.

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

http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc@104
PS3, Line 104: Unable to collect metrics to diagnostics log
Add a bit saying we'll retry in `kWaitBetweenFailures` seconds.


http://gerrit.cloudera.org:8080/#/c/9326/3/src/kudu/server/diagnostics_log.cc@118
PS3, Line 118: this seems redundant with the "only include changed metrics" 
above
Referring to `opts.only_modified_in_or_after_epoch` below?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f7bc7335b1f4c2a5c62fc5fe672289d5027a10d
Gerrit-Change-Number: 9326
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:27:06 +
Gerrit-HasComments: Yes

[kudu-CR] [TestWorkload] an option to retry on read timeouts

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9295 )

Change subject: [TestWorkload] an option to retry on read timeouts
..


Patch Set 4: Code-Review+2

agreed w/ Todd use of this may be indicative of scanner retry bugs


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54a6b81e4ba3a55676f1fc97cd577a5fe545c550
Gerrit-Change-Number: 9295
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:24:32 +
Gerrit-HasComments: No

[kudu-CR] Simplify OpId/Timestamp assignment and make it atomic

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7221 )

Change subject: Simplify OpId/Timestamp assignment and make it atomic
..


Patch Set 17:

(10 comments)

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

http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/consensus/quorum_util.cc@158
PS17, Line 158:   std::set uuids;
add: DCHECK(type == COMMITTED_CONFIG || type == PENDING_CONFIG)


http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/consensus/raft_consensus.h@449
PS17, Line 449: mus
must


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

http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/consensus/raft_consensus.cc@648
PS17, Line 648:   RETURN_NOT_OK(HandlePendingConfigChangeUnlocked(round));
> although this is right, it looks confusing because round probably isn't a c
or just: if (round->replicate_msg()->op_type() == CHANGE_CONFIG_OP) { ... }


http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/consensus/raft_consensus.cc@662
PS17, Line 662:   if (PREDICT_TRUE(round->replicate_msg()->op_type() != 
CHANGE_CONFIG_OP)) return Status::OK();
Due to the method name, seems less surprising to add: 
DCHECK_EQ(CHANGE_CONFIG_OP, round->replicate_msg()->op_type());


http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/consensus/raft_consensus.cc@673
PS17, Line 673:
nit: extra line


http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/consensus/raft_consensus.cc@691
PS17, Line 691: (
nit: no need for inner parentheses


http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/tablet/transactions/transaction_driver.cc
File src/kudu/tablet/transactions/transaction_driver.cc:

http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/tablet/transactions/transaction_driver.cc@336
PS17, Line 336: rule
s/rule/invariant/


http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/tablet/transactions/transaction_driver.cc@360
PS17, Line 360: constraint
s/constraint/above invariant/

By the way: is there a DCHECK somewhere in mvcc to enforce the invariant that 
clean time < any tx timestamp?


http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/tablet/transactions/transaction_driver.cc@371
PS17, Line 371: repl_state_copy
This variable masks a variable of the same name in an outer scope


http://gerrit.cloudera.org:8080/#/c/7221/17/src/kudu/tablet/transactions/transaction_driver.cc@384
PS17, Line 384: case REPLICATING
can we still get to this case?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2bedcc5c3708d06f48131f34ea14c835867b4800
Gerrit-Change-Number: 7221
Gerrit-PatchSet: 17
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 22:20:34 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9343 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9343/5/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/9343/5/src/kudu/rpc/rpc-test.cc@489
PS5, Line 489: controllers.back().get(), 
boost::bind(::CountDown, boost::ref(latch)));
> Yes you're right. I can't think of a sure shot way to make sure that we don
I still don't think that helps, because the RPCs will still end up read off the 
wire by the reactor thread and thus won't be pending in the client outbound 
queue, even if the server is blocked _processing them_. You'd have to somehow 
block the server's reactor thread so that it isn't reading off the pipe, and 
then you'd have to fill up the socket's send-buffer so that the client can't 
make progress sending the transfers to the pipe



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 21:55:09 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9343 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..


Patch Set 6:

> Build Failed
 >
 > http://jenkins.kudu.apache.org/job/kudu-gerrit/12071/ : FAILURE

Failure is in an unrelated flaky test.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 21:45:42 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2275. Upgrade libunwind to 1.3-rc1

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9335 )

Change subject: KUDU-2275. Upgrade libunwind to 1.3-rc1
..

KUDU-2275. Upgrade libunwind to 1.3-rc1

Per KUDU-2275, the version of libunwind that we were previously using
can occasionally crash during stack unwinding if it attempts to access a
page which is mprotected to be non-readable. This could be due to
incorrect interpretation of dwarf unwinding info or some other bug.

As we are trying to collect stacks more aggressively now, it's important
to upgrade. This new version uses a more robust mechanism for checking
whether a page is valid to access before accessing it during unwinding.

This also includes a patch from the libunwind git repo which fixes a
potential issue with libunwind in ASAN builds. I didn't run into this
issue yet myself, but seems like a straightforward and necessary fix.

Change-Id: I3f942a0d566e1b1e5ebe48f09362bf8aa3e1e33e
Reviewed-on: http://gerrit.cloudera.org:8080/9335
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/util/file_cache-test.cc
M thirdparty/download-thirdparty.sh
A 
thirdparty/patches/libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
M thirdparty/vars.sh
4 files changed, 61 insertions(+), 4 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3f942a0d566e1b1e5ebe48f09362bf8aa3e1e33e
Gerrit-Change-Number: 9335
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9343 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9343/5/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/9343/5/src/kudu/rpc/rpc-test.cc@489
PS5, Line 489: controllers.back().get(), 
boost::bind(::CountDown, boost::ref(latch)));
> Maybe I'm missing something here. If the main thread runs a bit slow before
Yes you're right. I can't think of a sure shot way to make sure that we don't 
have this race.

However, I've changed the RPC method to 
GenericCalculatorService::kSleepMethodName, with a 1 second sleep and 10 total 
RPCs. With 3 service threads (the default), that should give the main thread a 
little more than 3 seconds to dump the running RPCs information, which makes 
the flakiness very unlikely.

I'm open to suggestions if this doesn't suffice.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 21:21:46 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Lars Volker, Alexey Serbin, Dan Burkert, Kudu Jenkins, Todd 
Lipcon,

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

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

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

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..

KUDU-2301: (Part-1) Add instrumentation on a per connection level

This patch returns the OutboundTransfer queue size on a per
connection level and makes them accessible via the
DumpRunningRpcs() call.

A test is added in rpc-test to ensure that this metric works as
expected.

A future patch will add more metrics.

Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/connection.h
M src/kudu/rpc/messenger.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_introspection.proto
5 files changed, 66 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1704: add c++ client support for READ YOUR WRITES mode

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h@1740
PS9, Line 1740: /// When @c READ_YOUR_WRITES is specified, the server will 
pick a timestamp to use
I still think there is room to simplify these docs.  I would prefer if they 
didn't mention timestamps _at all_.


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h@1745
PS9, Line 1745: If no propagated timestamp is provided the server
What does this mean in terms of an application developer reading this doc?  How 
would such a developer provide a timestamp?  Why would such a developer want to 
provide a propagated timestamp?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 20 Feb 2018 20:51:58 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1704: add c++ client support for READ YOUR WRITES mode

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8823/8/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:

http://gerrit.cloudera.org:8080/#/c/8823/8/src/kudu/client/client-internal.h@278
PS8, Line 278:   AtomicInt max_observed_timestamp_;
> > As implemented here, isn't there interactions between two concurrently op
IIUC the RYW scan mode picks a propagated timestamp up-front, and then 
propagates that same timestamp with every NewScanRequest RPC.  To me, that 
implies that the timestamp should be kept in a field of the scanner, since it's 
a value specific to that scanner.

Instead it looks like it's being kept as a field on the client, and therefore 
is shared among potentially many scanners.  Isn't that problematic?

Edit: wrote the above, then read David's bullet-pointed comment.  I think he 
and I are basically getting to the same point.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 20 Feb 2018 20:47:44 +
Gerrit-HasComments: Yes

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

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8804 )

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


Patch Set 16:

(4 comments)

lgtm, just a nit and one last question

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2117
PS16, Line 2117: .
s/./,/


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2118
PS16, Line 2118: initiated
s/initiated with/default-constructed to/


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2118
PS16, Line 2118: Since
s/Since/since/


http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2163
PS16, Line 2163: (propagation ts + 1) should not
is that guaranteed by something?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48
Gerrit-Change-Number: 8804
Gerrit-PatchSet: 16
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 20 Feb 2018 20:47:44 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2275. Upgrade libunwind to 1.3-rc1

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9335 )

Change subject: KUDU-2275. Upgrade libunwind to 1.3-rc1
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f942a0d566e1b1e5ebe48f09362bf8aa3e1e33e
Gerrit-Change-Number: 9335
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 20 Feb 2018 20:29:28 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9343 )

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9343/5/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/9343/5/src/kudu/rpc/rpc-test.cc@489
PS5, Line 489:   
ASSERT_GT(dump_resp.outbound_connections(0).outbound_queue_size(), 0);
Maybe I'm missing something here. If the main thread runs a bit slow before 
DumpRunningRpcs calls, what is preventing the queue from immediately emptying 
before this assertion? eg if you added a sleep on line 486 wouldn't this test 
now fail? Even with 1000 RPCs it seems like they could very easily all get sent 
in a matter of a couple milliseconds



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 20:29:12 +
Gerrit-HasComments: Yes

[kudu-CR] Support decimal columns in load generation tool

[Grant Henke (Code Review)]

Grant Henke has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9362 )

Change subject: Support decimal columns in load generation tool
..

Support decimal columns in load generation tool

Change-Id: I8f9441ab3483775e74f5fdedfff309bdcbc8ab17
Reviewed-on: http://gerrit.cloudera.org:8080/9362
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_perf.cc
2 files changed, 23 insertions(+), 0 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8f9441ab3483775e74f5fdedfff309bdcbc8ab17
Gerrit-Change-Number: 9362
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1704: add c++ client support for READ YOUR WRITES mode

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 9:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client-internal.h@197
PS9, Line 197: from servers that only for all
 :   // opened scans with READ_YOUR_WRITES mode.
this sentence is non-sensical. also max/latest is bound to be confusing. remind 
me again why we need the two?
I thought we'd do something like:
- At the beginning of a scan, get the last propagated timestamp (say t0) and 
pass it to the scan config (we'll always use this one and won't read the last 
propagated timestamp from the client for the duration of the scan).
- With each scan request, for each tablet, send t0. Each server is free to 
choose a timestamp that is bigger than t0 exactly as implemented in your server 
side patch.
- Each scan response includes the chosen timestamp on the server, ts.
- Each scan response updates the "last propagated timestamp" with ts, making 
sure we have RYR.


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client-internal.h@201
PS9, Line 201:   // Updates the max observed timestamp from servers that only 
for all
 :   // opened scan with READ_YOUR_WRITES mode, if the given 
timestamp is
 :   // greater than the one stored.
 :   void UpdateMaxObservedTimestamp(uint64_t timestamp);
also non-sensical; also see my comment above


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

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client-test.cc@992
PS9, Line 992:  // Check counts changed accordingly using both READ_LATEST and
 :   // READ_YOUR_WRITES scan mode.
 :   for (auto read_mode : kReadModes) {
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 
kNoBound, kNoBound));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 
kNoBound, 15));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 27, 
kNoBound));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 0, 
15));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 0, 
10));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 0, 
20));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 0, 
30));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 14, 
30));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 30, 
30));
 : ASSERT_EQ(0, CountRowsFromClient(table.get(), read_mode, 
kTabletsNum * kRowsPerTablet,
 :  kNoBound));
 :   }
can you make this a parameterized test that received the read mode as input?


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/client.h@1743
PS9, Line 1743: minimize
nit: verbage (... ensures ... minimize sounds weird)


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/scan_token-internal.cc@253
PS9, Line 253: LOG(WARNING) << "Ignoring snapshot timestamp since not 
in "
 : "READ_AT_SNAPSHOT mode.";
should we validate this on the config? i.e. don't allow to pass a snapshot 
timestamp (or to have on set) when choosing RYW? we could return an error 
status there and then LOG(FATAL) here


http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/9/src/kudu/client/scanner-internal.cc@330
PS9, Line 330:   if (configuration_.has_snapshot_timestamp()) {
 : LOG(WARNING) << "Ignoring snapshot timestamp since "
 : "not in READ_AT_SNAPSHOT mode.";
 :   }
same comment I did elsewhere



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] internal mini cluster: support Cluster/LogVerifier

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9137 )

Change subject: internal_mini_cluster: support Cluster/LogVerifier
..


Patch Set 12: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228a6e3ba1a42db4e243ffdc5116f0c60ee04a84
Gerrit-Change-Number: 9137
Gerrit-PatchSet: 12
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 20:25:55 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2291 (part 2): Add a /stacks page

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9253 )

Change subject: KUDU-2291 (part 2): Add a /stacks page
..


Patch Set 5: Verified+1

unrelated consensus test flke


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b8f6d50d44e40fd51357fdbfd8f9ba2ebaa724b
Gerrit-Change-Number: 9253
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Tue, 20 Feb 2018 20:22:25 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2291 (part 2): Add a /stacks page

[Todd Lipcon (Code Review)]

Todd Lipcon has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/9253 )

Change subject: KUDU-2291 (part 2): Add a /stacks page
..


Removed reviewer Kudu Jenkins with the following votes:

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I8b8f6d50d44e40fd51357fdbfd8f9ba2ebaa724b
Gerrit-Change-Number: 9253
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] Support decimal columns in load generation tool

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9362 )

Change subject: Support decimal columns in load generation tool
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f9441ab3483775e74f5fdedfff309bdcbc8ab17
Gerrit-Change-Number: 9362
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 20 Feb 2018 18:17:38 +
Gerrit-HasComments: No

[kudu-CR] Support decimal columns in load generation tool

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9362 )

Change subject: Support decimal columns in load generation tool
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9362/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/9362/1/src/kudu/tools/kudu-tool-test.cc@1345
PS1, Line 1345: DECIMAL32
> DECIMAL64 ?
Done


http://gerrit.cloudera.org:8080/#/c/9362/1/src/kudu/tools/kudu-tool-test.cc@1348
PS1, Line 1348: DECIMAL32
> DECIMAL128 ?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f9441ab3483775e74f5fdedfff309bdcbc8ab17
Gerrit-Change-Number: 9362
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 20 Feb 2018 18:12:10 +
Gerrit-HasComments: Yes

[kudu-CR] Support decimal columns in load generation tool

[Grant Henke (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: Support decimal columns in load generation tool
..

Support decimal columns in load generation tool

Change-Id: I8f9441ab3483775e74f5fdedfff309bdcbc8ab17
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_perf.cc
2 files changed, 23 insertions(+), 0 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8f9441ab3483775e74f5fdedfff309bdcbc8ab17
Gerrit-Change-Number: 9362
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Support decimal columns in load generation tool

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9362 )

Change subject: Support decimal columns in load generation tool
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9362/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/9362/1/src/kudu/tools/kudu-tool-test.cc@1345
PS1, Line 1345: DECIMAL32
DECIMAL64 ?


http://gerrit.cloudera.org:8080/#/c/9362/1/src/kudu/tools/kudu-tool-test.cc@1348
PS1, Line 1348: DECIMAL32
DECIMAL128 ?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f9441ab3483775e74f5fdedfff309bdcbc8ab17
Gerrit-Change-Number: 9362
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 20 Feb 2018 18:05:12 +
Gerrit-HasComments: Yes

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

[Grant Henke (Code Review)]

Hello Mike Percy, Kudu Jenkins,

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

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

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

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

KUDU-721: [Flume] Add DECIMAL type support

Adds decimal column support to the Flume KuduSink
including the Regex and Avro operations producers.

Note: Sets enableDecimalLogicalType to true in the
Maven Avro plugin for code generation because the
default is false.

Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
---
M java/kudu-flume-sink/pom.xml
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
M java/kudu-flume-sink/src/test/avro/testAvroKuduOperationsProducer.avsc
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
6 files changed, 64 insertions(+), 23 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
Gerrit-Change-Number: 9365
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-2301: (Part-1) Add instrumentation on a per connection level

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Lars Volker, Alexey Serbin, Dan Burkert, Kudu Jenkins, Todd 
Lipcon,

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

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

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

Change subject: KUDU-2301: (Part-1) Add instrumentation on a per connection 
level
..

KUDU-2301: (Part-1) Add instrumentation on a per connection level

This patch returns the OutboundTransfer queue size on a per
connection level and makes them accessible via the
DumpRunningRpcs() call.

A test is added in rpc-test to ensure that this metric works as
expected.

A future patch will add more metrics.

Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/connection.h
M src/kudu/rpc/messenger.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_introspection.proto
5 files changed, 61 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae1a5fe0066adf644a9cac41ad6696e1bbf00465
Gerrit-Change-Number: 9343
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2305: Fix local variable usage to handle 2GB messages

[Joe McDonnell (Code Review)]

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9355 )

Change subject: KUDU-2305: Fix local variable usage to handle 2GB messages
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9355/3/src/kudu/rpc/serialization.cc
File src/kudu/rpc/serialization.cc:

http://gerrit.cloudera.org:8080/#/c/9355/3/src/kudu/rpc/serialization.cc@61
PS3, Line 61: static_cast(additional_size)
> Won't this be automatically promoted to int64_t ? Is this second cast neede
Yes, I think only the first cast is necessary to fix the ASAN issue. This seems 
like a code style thing, so I thought I'd let the Kudu guys comment. I don't 
have a preference.


http://gerrit.cloudera.org:8080/#/c/9355/3/src/kudu/rpc/transfer.cc
File src/kudu/rpc/transfer.cc:

http://gerrit.cloudera.org:8080/#/c/9355/3/src/kudu/rpc/transfer.cc@81
PS3, Line 81: kMsgLengthPrefixLength
> Does it make sense to to DCHECK_LE(kMsgLengthPrefixLength, INT_MAX) or in t
kMsgLengthPrefixLength is a uint8_t and is always 4, so rem is always a small 
number.

The comment right above this is confusing. I'll see if I can make it clearer.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e468099b16f7eb55de71b753acae8f57d18287c
Gerrit-Change-Number: 9355
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 20 Feb 2018 17:24:39 +
Gerrit-HasComments: Yes

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

[Grant Henke (Code Review)]

Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9365


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

KUDU-721: [Flume] Add DECIMAL type support

Adds decimal column support to the Flume KuduSink
including the Regex and Avro operations producers.

Change-Id: Ibc02d683dd1bce2cc0de255e4d072436b6c0163a
---
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducer.java
M 
java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducer.java
M java/kudu-flume-sink/src/test/avro/testAvroKuduOperationsProducer.avsc
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/AvroKuduOperationsProducerTest.java
M 
java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerTest.java
5 files changed, 61 insertions(+), 23 deletions(-)



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

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