[kudu-CR] KUDU-2290: Tool to re-create a tablet

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9393 )

Change subject: KUDU-2290: Tool to re-create a tablet
..


Patch Set 7:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/integration-tests/CMakeLists.txt
File src/kudu/integration-tests/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/integration-tests/CMakeLists.txt@101
PS7, Line 101: ADD_KUDU_TEST(replace_tablet-itest)
Could you follow the approach Todd took in commit 1c1d3ba to decide what value 
of PROCESSORS to assign to this new itest?


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/integration-tests/master-stress-test.cc
File src/kudu/integration-tests/master-stress-test.cc:

http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/integration-tests/master-stress-test.cc@297
PS7, Line 297: // The tablet is gone because it was already replaced or 
deleted.
Should we still wait in this case?


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/integration-tests/master-stress-test.cc@468
PS7, Line 468:   LOG(INFO) << "Tables created: " << num_tables_created_.Load();
What's the distribution of operations before and after your change? Curious how 
tablet replacement has affected things.


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/integration-tests/replace_tablet-itest.cc
File src/kudu/integration-tests/replace_tablet-itest.cc:

http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/integration-tests/replace_tablet-itest.cc@63
PS7, Line 63: CHECK(tablet_id);
Not needed?


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/integration-tests/replace_tablet-itest.cc@76
PS7, Line 76: CHECK(proxy);
Not needed?


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

http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/master/catalog_manager.cc@4324
PS7, Line 4324: new_tablet = scoped_refptr(new 
TabletInfo(table, GenerateId()));
How about:

  new_tablet.reset(new TabletInfo(...));


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/master/catalog_manager.cc@4346
PS7, Line 4346:   // Ensure we abort the mutations if persisting the changes 
fails.
Don't the l_table and l_tablets destructors abort automatically?


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/master/catalog_manager.cc@4358
PS7, Line 4358:   actions.tablets_to_delete.push_back(old_tablet);
Hmm, this should be in tablets_to_update, no? We never actually delete entries 
from the catalog table.


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/master/catalog_manager.cc@4376
PS7, Line 4376: table->AddRemoveTablets({new_tablet}, {old_tablet});
  : l_tablets.Commit();
  : l_table.Commit();
Are you sure these three should happen with the lock_ held? They acquire locks 
of their own; Commit() may also sleep waiting for locks, which we shouldn't do 
with a spinlock held.

IIRC other CatalogManager operations only hold lock_ to update global maps, 
then do the other steps without it.


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/master/catalog_manager.cc@4385
PS7, Line 4385: background_tasks_->Wake();
This is for the newly created tablet, right? Can you update the comment just 
above?


http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/tools/tool_action_common.h
File src/kudu/tools/tool_action_common.h:

http://gerrit.cloudera.org:8080/#/c/9393/7/src/kudu/tools/tool_action_common.h@155
PS7, Line 155:LeaderMasterProxy() = default;
Nit: indentation.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c
Gerrit-Change-Number: 9393
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 30 Mar 2018 04:37:28 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2393. Fix flaky DebugUtilTest.TestSnapshot

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2393. Fix flaky DebugUtilTest.TestSnapshot
..

KUDU-2393. Fix flaky DebugUtilTest.TestSnapshot

This test was flaky with two different failure modes which I believe
have the same root cause: when the test starts, it's possible that there
is still some thread from a prior test case in the process of shutting
down. Even though we join() on our threads religiously, there is a very
short period of time after join() returns where the dying thread is
still listed in /proc/self/task/. This meant that we would sometimes
fail the assertion that checks that the number of returned stacks
matches the number of running threads (because the thread finished
exitinng just before we called SnapshotAllStacks). In other cases we
would still see that exiting thread, but it would exit before responding
to our stack trace signal, so we'd see a "timed out" response.

The fix is a little hacky - we calculate the expected number of running
threads and ASSERT_EVENTUALLY to wait until reality matches
expectations.

Change-Id: I2f0fc29877b4d67a3720e47f4183684891f09bc0
---
M src/kudu/util/debug-util-test.cc
1 file changed, 30 insertions(+), 6 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f0fc29877b4d67a3720e47f4183684891f09bc0
Gerrit-Change-Number: 9864
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2361. Fix flaky MasterTest.TestDumpStacksOnRpcQueueOverflow

[Todd Lipcon (Code Review)]

Hello Mike Percy,

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

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

to review the following change.


Change subject: KUDU-2361. Fix flaky MasterTest.TestDumpStacksOnRpcQueueOverflow
..

KUDU-2361. Fix flaky MasterTest.TestDumpStacksOnRpcQueueOverflow

This test was flaky in TSAN builds for two reasons:

(1) even though taking the stack trace is always pretty fast, symbolization can
sometimes take more than a second. If that was the case, the assertion that
looked at the diagnostics log content would fail just because the expected log
was not there _yet_. Fixing this issue using ASSERT_EVENTUALLY brought the test
failure rate with 4 stress threads down from about 60% to about 0.3%.

(2) with periodic stack dumping enabled, it was possible that the periodic dump
was already in progress at the time that the overflow-triggered dump was
triggered. In that case, the overflow-triggered dump would be ignored. That
was expected behavior, but the test would fail due to not seeing the overflow
listed as the "reason". This patch fixes this issue by disabling periodic
dumping in the test.

In order to disable periodic dumping but leave triggered dumping enabled,
I changed the semantics of the configuration slightly. Previously, setting
it to '0' would disable all dumping. Now, setting it to '0' disables
periodic dumping but still leaves triggered dumps enabled. Setting it to
'-1' disables all dumping.

With both these changes I was able to loop the test 300 times with 4 stress
threads in TSAN mode with no failures.

Change-Id: If7078bc87d2cf724d5f2fddf1173ac1786279e7e
---
M src/kudu/master/master-test.cc
M src/kudu/server/diagnostics_log.cc
2 files changed, 26 insertions(+), 10 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If7078bc87d2cf724d5f2fddf1173ac1786279e7e
Gerrit-Change-Number: 9867
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] KUDU-2388. Fix TSAN race in MaintenanceManager::UnregisterOp

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves,

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

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

to review the following change.


Change subject: KUDU-2388. Fix TSAN race in MaintenanceManager::UnregisterOp
..

KUDU-2388. Fix TSAN race in MaintenanceManager::UnregisterOp

This fixes the following TSAN race:

WARNING: ThreadSanitizer: data race (pid=17822)  Read of size 1 at 
0x7b4c54e8 by thread T59 (mutexes: write M1750):
...
#3 
strings::internal::SubstituteArg::SubstituteArg(std::__1::basic_string const&) 
/data/somelongdirectorytoavoidrpathissues/src/kudu/src/kudu/gutil/strings/substitute.h:76
 (libtserver.so+0x9edb0)
#4 kudu::MaintenanceManager::LogPrefix() const 
/data/somelongdirectorytoavoidrpathissues/src/kudu/src/kudu/util/maintenance_manager.cc:545:31
 (libkudu_util.so+0x167791)
#5 kudu::MaintenanceManager::UnregisterOp(kudu::MaintenanceOp*) 
/data/somelongdirectorytoavoidrpathissues/src/kudu/src/kudu/util/maintenance_manager.cc:235:3
 (libkudu_util.so+0x165963)
#6 kudu::MaintenanceOp::Unregister() 
/data/somelongdirectorytoavoidrpathissues/src/kudu/src/kudu/util/maintenance_manager.cc:123:13
 (libkudu_util.so+0x1654fe)
#7 kudu::tablet::Tablet::UnregisterMaintenanceOps() 
/data/somelongdirectorytoavoidrpathissues/src/kudu/src/kudu/tablet/tablet.cc:1405:9
 (libtablet.so+0xfb5af)
#8 kudu::tablet::TabletReplica::Stop() 
/data/somelongdirectorytoavoidrpathissues/src/kudu/src/kudu/tablet/tablet_replica.cc:271:25
 (libtablet.so+0x146e66)
#9 
kudu::tserver::TSTabletManager::DeleteTablet(std::__1::basic_string c

  Previous write of size 8 at 0x7b4c54e8 by main thread:
#0 memcpy 
/data/somelongdirectorytoavoidrpathissues/src/kudu/thirdparty/src/llvm-4.0.0.src/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_common_interceptors.inc:655
 (kudu-tserver+0x449e4c)
#1 std::__1::basic_string::__move_assign(std::__1::basic_string&, 
std::__1::integral_constant) 
/data/somelongdirectorytoavoidrpathissues/src/kudu/thirdparty/installed/tsan/include/c++/v1/string:2044:18
 (libkudu_util.so+0x16664d)
#2 std::__1::basic_string::operator=(std::__1::basic_string&&) 
/data/somelongdirectorytoavoidrpathissues/src/kudu/thirdparty/installed/tsan/include/c++/v1/string:2055
 (libkudu_util.so+0x16664d)
#3 kudu::MaintenanceManager::Init(std::__1::basic_string) 
/data/somelongdirectorytoavoidrpathissues/src/kudu/src/kudu/util/maintenance_manager.cc:169
 (libkudu_util.so+0x16664d)
...

The race is on the 'server_uuid_' field in the MaintenanceManager. This is set
during startup, but was being set _after_ calls such as UnregisterOp could be
made as seen above. That means the UnregisterOp call could either see an empty
UUID or even crash due to the above race.

This simply rejiggers the MaintenanceManager startup to take the UUID in as a
constructor parameter instead, and to instantiate the object slightly later
during startup.

Change-Id: Id06731f56eb98146f7b88541b936c0026b781b16
---
M src/kudu/master/master.cc
M src/kudu/tserver/tablet_server.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/maintenance_manager.h
5 files changed, 24 insertions(+), 16 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id06731f56eb98146f7b88541b936c0026b781b16
Gerrit-Change-Number: 9866
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 

[kudu-CR] KUDU-2393. Fix flaky DebugUtilTest.TestSnapshot

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2393. Fix flaky DebugUtilTest.TestSnapshot
..


Patch Set 1:

I looped this 100 times prior to the fix, with --stress-cpu-threads=4, and it 
failed 4%. I then looped with the fix 400 times and it failed 0.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0fc29877b4d67a3720e47f4183684891f09bc0
Gerrit-Change-Number: 9864
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 02:59:27 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (8/n): Integrate HmsCatalog into CatalogManager

[Dan Burkert (Code Review)]

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

Change subject: KUDU-2191 (8/n): Integrate HmsCatalog into CatalogManager
..


Patch Set 2:

This was split out from https://gerrit.cloudera.org/c/8312/.  It contains the 
CatalogManager integration with HmsCatalog.  Nothing substantial has changed 
since that change list.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie68e143c3c317c7690af097e6485934feb1010b4
Gerrit-Change-Number: 9863
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 02:45:34 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (7/n): HmsCatalog

[Dan Burkert (Code Review)]

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

Change subject: KUDU-2191 (7/n): HmsCatalog
..


Patch Set 2:

And a bunch of tests were added to hms_catalog-test


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6eb2a4c400f5aaee095e4e4ad572981565a1c040
Gerrit-Change-Number: 9862
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 02:45:03 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (7/n): HmsCatalog

[Dan Burkert (Code Review)]

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

Change subject: KUDU-2191 (7/n): HmsCatalog
..


Patch Set 2:

This was split out from https://gerrit.cloudera.org/c/8312/.  It contains the 
HmsCatalog abstraction.  The gflags and especially the handling of 
hive_metastore_uris has changed since the previous change list.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6eb2a4c400f5aaee095e4e4ad572981565a1c040
Gerrit-Change-Number: 9862
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 02:44:44 +
Gerrit-HasComments: No

[kudu-CR] WIP: flags: add API for thread-safe reading of string flags

[Todd Lipcon (Code Review)]

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


Change subject: WIP: flags: add API for thread-safe reading of string flags
..

WIP: flags: add API for thread-safe reading of string flags

Accessing FLAGS_foo when 'foo' is a string flag is not safe with
concurrent modification. As such, we have tended to not make any of our
string flags runtime-modifiable. It should be safe, however, to modify
such flags at runtime if we use the programmatic flag accessor.

This adds utility functions for doing so.

WIP because I don't have any call sites -- wrote this but didn't end up
needing to use it.

Change-Id: I588f7f27b8df7e3fc388b791d72ef04e1969a788
---
M src/kudu/util/flags.cc
M src/kudu/util/flags.h
2 files changed, 37 insertions(+), 0 deletions(-)



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

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

[kudu-CR] KUDU-2393. Fix flaky DebugUtilTest.TestSnapshot

[Todd Lipcon (Code Review)]

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


Change subject: KUDU-2393. Fix flaky DebugUtilTest.TestSnapshot
..

KUDU-2393. Fix flaky DebugUtilTest.TestSnapshot

This test was flaky with two different failure modes which I believe
have the same root cause: when the test starts, it's possible that there
is still some thread from a prior test case in the process of shutting
down. Even though we join() on our threads religiously, there is a very
short period of time after join() returns where the dying thread is
still listed in /proc/self/task/. This meant that we would sometimes
fail the assertion that checks that the number of returned stacks
matches the number of running threads (because the thread finished
exitinng just before we called SnapshotAllStacks). In other cases we
would still see that exiting thread, but it would exit before responding
to our stack trace signal, so we'd see a "timed out" response.

The fix is a little hacky - we calculate the expected number of running
threads and ASSERT_EVENTUALLY to wait until reality matches
expectations.

Change-Id: I2f0fc29877b4d67a3720e47f4183684891f09bc0
---
M src/kudu/util/debug-util-test.cc
1 file changed, 32 insertions(+), 6 deletions(-)



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

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

[kudu-CR] KUDU-2191 (6/n): Fixups to the C++ HMS client

[Dan Burkert (Code Review)]

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

Change subject: KUDU-2191 (6/n): Fixups to the C++ HMS client
..


Patch Set 1:

This was split out from https://gerrit.cloudera.org/c/8312/, it contains the 
changes to hms_client.h/cc.  I don't think it contains new that the other 
review didn't.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86a8d984e67fe6b4c004725828b4296476c2389e
Gerrit-Change-Number: 9861
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 02:43:21 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (7/n): HmsCatalog

[Dan Burkert (Code Review)]

Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao, Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (7/n): HmsCatalog
..

KUDU-2191 (7/n): HmsCatalog

This commit adds some important higher-level HMS machinery in the
HmsCatalog class.  This class is responsible for translating Kudu
catalog information to the Hive format, thread safety, retries, Hive HA,
and configuration flags for the Hive integration.

A follow-up commit will integrate the HmsCatalog into the
CatalogManager, at which point many parts of the HMS integration will
become functional. Until that point this is still non-functional code.

Change-Id: I6eb2a4c400f5aaee095e4e4ad572981565a1c040
---
M src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hms_catalog-test.cc
A src/kudu/hms/hms_catalog.cc
A src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/master/CMakeLists.txt
M src/kudu/master/master.cc
8 files changed, 1,090 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6eb2a4c400f5aaee095e4e4ad572981565a1c040
Gerrit-Change-Number: 9862
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2191 (8/n): Integrate HmsCatalog into CatalogManager

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Adar Dembo, Hao Hao, Todd Lipcon,

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

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

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

Change subject: KUDU-2191 (8/n): Integrate HmsCatalog into CatalogManager
..

KUDU-2191 (8/n): Integrate HmsCatalog into CatalogManager

This commit connects the CatalogManager to the HMS via the HmsCatalog
abstraction. When the HMS integration is enabled (by setting the
--hive-metastore-uris flag) and Kudu tables are created, altered, or
dropped, the corresponding HMS entry is also modified appropriately.
Additionally, New table and column names are required to be valid
according to the Hive identifier rules, which are much stricter than
Kudu's existing identifier rules.

Testing: This commit adds a new integration test (master_hms-itest)
which tests that the integration works as expected with
create/alter/drop table operations. Additionally, some existing DDL
stress tests now have the HMS integration enabled in order to provide
more coverage.

Change-Id: Ie68e143c3c317c7690af097e6485934feb1010b4
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/master-stress-test.cc
M src/kudu/integration-tests/master_failover-itest.cc
A src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/mini-cluster/external_mini_cluster.cc
12 files changed, 603 insertions(+), 45 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie68e143c3c317c7690af097e6485934feb1010b4
Gerrit-Change-Number: 9863
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2384. Don't collect thread stacks when running under debugger

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9835 )

Change subject: KUDU-2384. Don't collect thread stacks when running under 
debugger
..


Patch Set 3:

> This is an interesting case. fs_manager-test is failing because 
> KernelStackWatchdog continues to run in the background after the gflags 
> destructor runs, and now KernelStackWatchdog calls Env::ReadFileToString 
> which checks the fault injection string flag, and that string has been 
> destructed.
>
> So, it seems the potential fixes are:
> (a) make that test case reset fault injection in its destructor
> (b) make KernelStackWatchdog avoid using Env calls that are subject to fault 
> injection
> (c) make fault injection short circuit to "not enabled" for /proc reads 
> (which perhaps is more accurate)
>
> Thoughts?

I'd be in favor of (c). (a) is unintuitive, and can easily happen again in 
other tests that turn on fault injection.

That said, the test in question (TestCreateWithFailedDirs) already resets 
env_inject_eio to 0 when it finishes; if it didn't, we'd also see a warning 
that we couldn't clean up the test's temp dir. But I guess here the issue isn't 
with env_inject_eio, but with env_inject_eio_globs? We don't typically reset 
that one.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib17ffb9514e9b440b8d97843374d2a60913a398b
Gerrit-Change-Number: 9835
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 02:19:19 +
Gerrit-HasComments: No

[kudu-CR] KUDU-16 pt 2: add client-side limits on scanners

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-16 pt 2: add client-side limits on scanners
..


Patch Set 4:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9790/4/src/kudu/client/client-test.cc@812
PS4, Line 812: TEST_F(ClientTest, TestRandomizedLimitScans) {
If I'm reading client-test correctly, this only sets up a table with two 
tablets, and the first tablet only has 9 rows in it. Perhaps we should have 
some test which uses hash partitioning and ensures that each tablet has more 
than one batch worth of rows?


http://gerrit.cloudera.org:8080/#/c/9790/4/src/kudu/client/scan_configuration.h
File src/kudu/client/scan_configuration.h:

http://gerrit.cloudera.org:8080/#/c/9790/4/src/kudu/client/scan_configuration.h@153
PS4, Line 153: return has_limit_;
hm, why not use boost::optional here?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2d40e3d14e36f3bf1d09a4bfdb3e17a745d244d
Gerrit-Change-Number: 9790
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 02:07:26 +
Gerrit-HasComments: Yes

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

[Todd Lipcon (Code Review)]

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

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


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9372/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9372/6//COMMIT_MSG@29
PS6, Line 29: Reading from blocks (s):  40.578  38.428  37.093  
38.700
This seems to be missing the punch line. Is this implying that, with the patch 
active, we would cut out this 38sec entirely because we wouldn't read from 
blocks anymore?



--
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: comment
Gerrit-Change-Id: I37d6f7160e3a7188753684e063963110f70e9b8d
Gerrit-Change-Number: 9372
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 02:00:15 +
Gerrit-HasComments: Yes

[kudu-CR] rpc-test: fix multi-threaded test from running too long

[Todd Lipcon (Code Review)]

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

Change subject: rpc-test: fix multi-threaded test from running too long
..

rpc-test: fix multi-threaded test from running too long

In TSAN builds this test was taking several minutes. This makes the test
run based on a timer rather than an explicit count.

Change-Id: I827a07d5b1d2463cb845ef6b04ec36036b2ddfbb
Reviewed-on: http://gerrit.cloudera.org:8080/9856
Reviewed-by: Sailesh Mukil 
Reviewed-by: Adar Dembo 
Tested-by: Todd Lipcon 
---
M src/kudu/rpc/rpc-test.cc
1 file changed, 6 insertions(+), 2 deletions(-)

Approvals:
  Sailesh Mukil: Looks good to me, but someone else must approve
  Adar Dembo: Looks good to me, approved
  Todd Lipcon: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I827a07d5b1d2463cb845ef6b04ec36036b2ddfbb
Gerrit-Change-Number: 9856
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] rpc-test: fix multi-threaded test from running too long

[Todd Lipcon (Code Review)]

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

Change subject: rpc-test: fix multi-threaded test from running too long
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I827a07d5b1d2463cb845ef6b04ec36036b2ddfbb
Gerrit-Change-Number: 9856
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:58:03 +
Gerrit-HasComments: No

[kudu-CR] rpc-test: fix multi-threaded test from running too long

[Todd Lipcon (Code Review)]

Todd Lipcon has removed a vote on this change.

Change subject: rpc-test: fix multi-threaded test from running too long
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I827a07d5b1d2463cb845ef6b04ec36036b2ddfbb
Gerrit-Change-Number: 9856
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

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

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 4: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:56:59 +
Gerrit-HasComments: No

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Todd Lipcon has removed a vote on this change.

Change subject: meta_cache: improve multi-threaded scalability
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-16 pt 1: add server-side limits on scanners

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-16 pt 1: add server-side limits on scanners
..


Patch Set 9:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/common/wire_protocol-test.cc
File src/kudu/common/wire_protocol-test.cc:

http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/common/wire_protocol-test.cc@323
PS9, Line 323: boost::none /* max_num_rows_to_return */, 
true /* pad timestamps */);
if you write this as: /* max_num_rows_to_return= */boost::none then clang-tidy 
will check that the param name matches


http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:

http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/common/wire_protocol.cc@762
PS9, Line 762: max_num_rows_to_return
we can get rid of some branching here by doing this once up front:

int max_rows = max_num_rows_to_return.value_or(block.nrows());

(this is a very hot function on the scan path)


http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/common/wire_protocol.cc@771
PS9, Line 771: (*max_num_rows_to_return - rows_returned)
can also avoid this by updating a 'max_rows' above incrementally


http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/common/wire_protocol.cc@838
PS9, Line 838: num_rows
what's the purpose of allowing this to be negative in this function? can't it 
just be checked at a higher level and be a DCHECK here to simplify some stuff a 
bit?


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

http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/tserver/tablet_server-test.cc@2072
PS9, Line 2072: lmits
typo


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

http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/tserver/tablet_service.cc@485
PS9, Line 485: optional
maybe this can be non-optional and simplify code paths by just setting it to 
row_block.nrows() here and get rid of the optional processing deeper down?


http://gerrit.cloudera.org:8080/#/c/9783/9/src/kudu/tserver/tablet_service.cc@502
PS9, Line 502: num_rows_returned_
do we still need this member if it's redundant with the one inside the scannner?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ca5ddff09f4910d12f80259bd11e4027f99aa7c
Gerrit-Change-Number: 9783
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:53:48 +
Gerrit-HasComments: Yes

[kudu-CR] [examples] Update collectl example

[Todd Lipcon (Code Review)]

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

Change subject: [examples] Update collectl example
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9854/1/examples/java/collectl/README.adoc
File examples/java/collectl/README.adoc:

http://gerrit.cloudera.org:8080/#/c/9854/1/examples/java/collectl/README.adoc@27
PS1, Line 27: pre-defined schema.
do you think it's worth adding something here like:

NOTE: this code is meant as an example of Java API usage and is not meant to be 
a full-featured solution for storing metrics.

... or something to that effect? eg if someone submitted a bunch of patches to 
make this into a "better collectl storage" we might reject them since the idea 
is to be a condensed example rather than something useful on its own.


http://gerrit.cloudera.org:8080/#/c/9854/1/examples/java/collectl/README.adoc@97
PS1, Line 97: 0
maybe update to scala 2.11 and kudu 1.7?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5d0385e51724f1f462e059603f983a3f3408d96
Gerrit-Change-Number: 9854
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:42:34 +
Gerrit-HasComments: Yes

[kudu-CR] [examples] Improve the basic Java example

[Todd Lipcon (Code Review)]

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

Change subject: [examples] Improve the basic Java example
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9852/1/examples/java/java-example/src/main/java/org/apache/kudu/examples/Example.java
File 
examples/java/java-example/src/main/java/org/apache/kudu/examples/Example.java:

http://gerrit.cloudera.org:8080/#/c/9852/1/examples/java/java-example/src/main/java/org/apache/kudu/examples/Example.java@75
PS1, Line 75:   cto.setRangePartitionColumns(rangeKeys);
Now that we support hash partitioning, and we kinda think that hash 
partitioning is a better starting choice, perhaps we should be using hash 
partitioning in our simple example?


http://gerrit.cloudera.org:8080/#/c/9852/1/examples/java/java-example/src/main/java/org/apache/kudu/examples/Example.java@129
PS1, Line 129:   // Scan with a predicate on the 'key' column, returning 
the 'value' and "added" columns.
maybe we can refactor out some methods from this enormous function? eg 'void 
exampleScan()' and 'void setupTable', etc



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79ab5b0b2c30e8fd7c799857a0e3754b7618a580
Gerrit-Change-Number: 9852
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:39:47 +
Gerrit-HasComments: Yes

[kudu-CR] Improve the C++ client example

[Todd Lipcon (Code Review)]

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

Change subject: Improve the C++ client example
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9838/1/samples/cpp/sample.cc
File samples/cpp/sample.cc:

http://gerrit.cloudera.org:8080/#/c/9838/1/samples/cpp/sample.cc@297
PS1, Line 297:   if (exists) {
> Whoa there Tidy Bot. Everything is going to be ok.
yea, clang doesn't do inter-translation-unit analysis, so it has no idea that 
there's a contract that pointer out-params are modified. It's too bad it 
doesn't take a more conservative stance and assume that they are.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7fcc9f46b5e28b18f1a96082fd61a1f3402f59ff
Gerrit-Change-Number: 9838
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:34:18 +
Gerrit-HasComments: Yes

[kudu-CR] util: use uint64 t values for resource limits and adjust usages

[Todd Lipcon (Code Review)]

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

Change subject: util: use uint64_t values for resource limits and adjust usages
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e9bb71af886f732f36a8c93876f47e7591aeadb
Gerrit-Change-Number: 9806
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:29:33 +
Gerrit-HasComments: No

[kudu-CR] util: use uint64 t values for resource limits and adjust usages

[Todd Lipcon (Code Review)]

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

Change subject: util: use uint64_t values for resource limits and adjust usages
..

util: use uint64_t values for resource limits and adjust usages

Turns out rlim_t is an unsigned 64-bit type, and although a root shell on my
laptop won't let me set RLIMIT_NOFILE particularly high, I can set
RLIMIT_NPROC to any value, and even to RLIM_INFINITY (-1).

So to handle every value correctly, we should really be treating these as
uint64_t rather than int64_t. That also gives us correct RLIM_INFINITY
handling for free, because -1 becomes kuint64max. This also effectively
reverts commit 17aceda.

However, not all callers expect uint64_t values, so some conversion is still
necessary. I also took the opportunity to beef up how the resource limits
were used by incorporating the values of /proc/sys/kernel/pid_max,
/proc/sys/kernel/threads-max, and /proc/sys/fs/file-max as upper bounds.

Change-Id: I7e9bb71af886f732f36a8c93876f47e7591aeadb
Reviewed-on: http://gerrit.cloudera.org:8080/9806
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/fs/block_manager.cc
M src/kudu/kserver/kserver.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
4 files changed, 58 insertions(+), 29 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7e9bb71af886f732f36a8c93876f47e7591aeadb
Gerrit-Change-Number: 9806
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2289: Tablet deletion should be throttled

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2289: Tablet deletion should be throttled
..


Patch Set 5:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9551/5/src/kudu/integration-tests/delete_table-itest.cc@1191
PS5, Line 1191:
  :   // Despite sending more DeleteTablet requests than (service 
queue length + num service threads),
  :   // the master should never have had to retry a DeleteTablet 
request.
> All good points. Perhaps bug Mike or Todd for ideas? If no one can suggest
yea, I'm not convinced this needs a separate unit test since it's sort of 
"correct by construction" given that we've separately tested thread pools, etc.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3819bf8a3acf8ea03a76cc6cacd92d85bb114998
Gerrit-Change-Number: 9551
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:27:08 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (6/n): Hive Metastore catalog manager integration

[Dan Burkert (Code Review)]

Dan Burkert has abandoned this change. ( http://gerrit.cloudera.org:8080/8312 )

Change subject: KUDU-2191 (6/n): Hive Metastore catalog manager integration
..


Abandoned

split into smaller chunks
--
To view, visit http://gerrit.cloudera.org:8080/8312
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf
Gerrit-Change-Number: 8312
Gerrit-PatchSet: 10
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2191 (6/n): Hive Metastore catalog manager integration

[Dan Burkert (Code Review)]

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

Change subject: KUDU-2191 (6/n): Hive Metastore catalog manager integration
..


Patch Set 10:

> Patch Set 10:
>
> (6 comments)

This has been split into the following reviews:
https://gerrit.cloudera.org/c/9861/
https://gerrit.cloudera.org/c/9862/
https://gerrit.cloudera.org/c/9863/


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf
Gerrit-Change-Number: 8312
Gerrit-PatchSet: 10
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 30 Mar 2018 01:26:26 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2191 (7/n): HmsCatalog

[Dan Burkert (Code Review)]

Dan Burkert has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9862


Change subject: KUDU-2191 (7/n): HmsCatalog
..

KUDU-2191 (7/n): HmsCatalog

This commit adds some important higher-level HMS machinery in the
HmsCatalog class.  This class is responsible for translating Kudu
catalog information to the Hive format, thread safety, retries, Hive HA,
and configuration flags for the Hive integration.

A follow-up commit will integrate the HmsCatalog into the
CatalogManager, at which point many parts of the HMS integration will
become functional. Until that point this is still non-functional code.

Change-Id: I6eb2a4c400f5aaee095e4e4ad572981565a1c040
---
M src/kudu/hms/CMakeLists.txt
A src/kudu/hms/hms_catalog-test.cc
A src/kudu/hms/hms_catalog.cc
A src/kudu/hms/hms_catalog.h
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/master/CMakeLists.txt
M src/kudu/master/master.cc
8 files changed, 1,093 insertions(+), 2 deletions(-)



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

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

[kudu-CR] KUDU-2191 (6/n): Fixups to the C++ HMS client

[Dan Burkert (Code Review)]

Dan Burkert has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9861


Change subject: KUDU-2191 (6/n): Fixups to the C++ HMS client
..

KUDU-2191 (6/n): Fixups to the C++ HMS client

These are changes necessary for subsequent patches in the series, but
they are easier to review in isolation.

Change-Id: I86a8d984e67fe6b4c004725828b4296476c2389e
---
M 
java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/hms_client.h
M src/kudu/hms/mini_hms.cc
5 files changed, 80 insertions(+), 12 deletions(-)



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

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

[kudu-CR] KUDU-2191 (8/n): Integrate HmsCatalog into CatalogManager

[Dan Burkert (Code Review)]

Dan Burkert has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9863


Change subject: KUDU-2191 (8/n): Integrate HmsCatalog into CatalogManager
..

KUDU-2191 (8/n): Integrate HmsCatalog into CatalogManager

This commit connects the CatalogManager to the HMS via the HmsCatalog
abstraction. When the HMS integration is enabled (by setting the
--hive-metastore-uris flag) and Kudu tables are created, altered, or
dropped, the corresponding HMS entry is also modified appropriately.
Additionally, New table and column names are required to be valid
according to the Hive identifier rules, which are much stricter than
Kudu's existing identifier rules.

Testing: This commit adds a new integration test (master_hms-itest)
which tests that the integration works as expected with
create/alter/drop table operations. Additionally, some existing DDL
stress tests now have the HMS integration enabled in order to provide
more coverage.

Change-Id: Ie68e143c3c317c7690af097e6485934feb1010b4
---
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/master-stress-test.cc
M src/kudu/integration-tests/master_failover-itest.cc
A src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/mini-cluster/external_mini_cluster.cc
12 files changed, 604 insertions(+), 45 deletions(-)



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

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

[kudu-CR] [examples] Improve loadgen example

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9853 )

Change subject: [examples] Improve loadgen example
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9853/1/examples/java/insert-loadgen/README.adoc
File examples/java/insert-loadgen/README.adoc:

http://gerrit.cloudera.org:8080/#/c/9853/1/examples/java/insert-loadgen/README.adoc@45
PS1, Line 45: create-demo-table
This doesn't exist anymore.


http://gerrit.cloudera.org:8080/#/c/9853/1/examples/java/insert-loadgen/src/main/java/org/apache/kudu/examples/InsertLoadgen.java
File 
examples/java/insert-loadgen/src/main/java/org/apache/kudu/examples/InsertLoadgen.java:

http://gerrit.cloudera.org:8080/#/c/9853/1/examples/java/insert-loadgen/src/main/java/org/apache/kudu/examples/InsertLoadgen.java@126
PS1, Line 126:   session.flush();
Hmm, if we're using AUTO_FLUSH_BACKGROUND why do we need this explicit flush?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc0e655f56adcedec2c3dd8a8b3c900e3b2e7803
Gerrit-Change-Number: 9853
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 30 Mar 2018 00:01:13 +
Gerrit-HasComments: Yes

[kudu-CR] [examples] Improve the basic Java example

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9852 )

Change subject: [examples] Improve the basic Java example
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9852/1/examples/java/java-example/pom.xml
File examples/java/java-example/pom.xml:

http://gerrit.cloudera.org:8080/#/c/9852/1/examples/java/java-example/pom.xml@24
PS1, Line 24:   kudu-examples
Hmm, perhaps we should be publishing into org.apache.kudu, just like the rest 
of our artifacts? Or would we never publish the examples to a Maven repo?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79ab5b0b2c30e8fd7c799857a0e3754b7618a580
Gerrit-Change-Number: 9852
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 29 Mar 2018 23:59:29 +
Gerrit-HasComments: Yes

[kudu-CR] [examples] Update collectl example

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9854 )

Change subject: [examples] Update collectl example
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9854/1/examples/java/collectl/README.adoc
File examples/java/collectl/README.adoc:

http://gerrit.cloudera.org:8080/#/c/9854/1/examples/java/collectl/README.adoc@60
PS1, Line 60: CREATE EXTERNAL TABLE `metrics` (
I imagine the CREATE TABLE syntax has changed since this was first written.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5d0385e51724f1f462e059603f983a3f3408d96
Gerrit-Change-Number: 9854
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 29 Mar 2018 23:56:49 +
Gerrit-HasComments: Yes

[kudu-CR] [examples] Add licenses to Python examples

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9858 )

Change subject: [examples] Add licenses to Python examples
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9858/1/examples/python/graphite-kudu/README.adoc
File examples/python/graphite-kudu/README.adoc:

http://gerrit.cloudera.org:8080/#/c/9858/1/examples/python/graphite-kudu/README.adoc@22
PS1, Line 22: is
drop


http://gerrit.cloudera.org:8080/#/c/9858/1/examples/python/graphite-kudu/README.adoc@23
PS1, Line 23: a
an



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8825bfac0416fc19fdb99c972eb104898ba78e7d
Gerrit-Change-Number: 9858
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 29 Mar 2018 23:54:09 +
Gerrit-HasComments: Yes

[kudu-CR] [examples] Improve the basic Java example

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9852 )

Change subject: [examples] Improve the basic Java example
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9852/1/examples/java/java-example/src/main/java/org/apache/kudu/examples/Example.java
File 
examples/java/java-example/src/main/java/org/apache/kudu/examples/Example.java:

http://gerrit.cloudera.org:8080/#/c/9852/1/examples/java/java-example/src/main/java/org/apache/kudu/examples/Example.java@89
PS1, Line 89:   KuduSession session = client.newSession();
I wonder if it'd be useful to demonstrate or at least draw some attention to 
setting the flush mode. WDYT? It might seem a bit in the weeds, but it seems 
like one of a few client-knobs/levers not already included in this example, and 
maybe we should at least call out all of basic the knobs/levers. In the same 
spirit, maybe also make one of the columns nullable? And add hash partitioning?

As a developer looking for a quick how-to without e.g. digging through API docs 
or source code, getting a taste for all the things in one example seems like 
it'd be super convenient, at the cost of a few more lines.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79ab5b0b2c30e8fd7c799857a0e3754b7618a580
Gerrit-Change-Number: 9852
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 29 Mar 2018 23:40:19 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-16 pt 2: add client-side limits on scanners

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9790 )

Change subject: KUDU-16 pt 2: add client-side limits on scanners
..


Patch Set 4:

> Patch Set 1:
>
> Haven't fully reviewed this patch, just looked at the tests, overall.
> I think we need a more randomized test that scans from both memrowsets and 
> diskrowsets and has arbitrary limits.

Done


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib2d40e3d14e36f3bf1d09a4bfdb3e17a745d244d
Gerrit-Change-Number: 9790
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 29 Mar 2018 23:25:13 +
Gerrit-HasComments: No

[kudu-CR] KUDU-16 pt 1: add server-side limits on scanners

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9783 )

Change subject: KUDU-16 pt 1: add server-side limits on scanners
..


Patch Set 9:

The pre-commit failures appear unrelated.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ca5ddff09f4910d12f80259bd11e4027f99aa7c
Gerrit-Change-Number: 9783
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Thu, 29 Mar 2018 23:24:57 +
Gerrit-HasComments: No

[kudu-CR] KUDU-16 pt 2: add client-side limits on scanners

[Andrew Wong (Code Review)]

Hello David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: KUDU-16 pt 2: add client-side limits on scanners
..

KUDU-16 pt 2: add client-side limits on scanners

This patch adds a public API to allow the specification of
per-client-side-scanner limits on the number of rows returned. Each
scanner will maintain a count of the number of rows already read, and
adjust the server-side limit upon sending the next scan request.

A couple of tests are included to verify that the limits act as
expected. I also verified that lowering the limit reduces the number of
bytes read on disk (at the granularity of a single scan batch at a
time).

Note: this patch does not implement the behavior for scan tokens.

Change-Id: Ib2d40e3d14e36f3bf1d09a4bfdb3e17a745d244d
---
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/scanner-internal.cc
M src/kudu/client/scanner-internal.h
7 files changed, 185 insertions(+), 40 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib2d40e3d14e36f3bf1d09a4bfdb3e17a745d244d
Gerrit-Change-Number: 9790
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-16 pt 1: add server-side limits on scanners

[Andrew Wong (Code Review)]

Hello Tidy Bot, Dan Burkert, David Ribeiro Alves, Kudu Jenkins, Adar Dembo, 
Grant Henke,

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

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

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

Change subject: KUDU-16 pt 1: add server-side limits on scanners
..

KUDU-16 pt 1: add server-side limits on scanners

This patch adds the functionality to specify limits to the number of
rows returned per tablet server scanner. Internally, each server-side
scanner now tracks the number of rows it has already returned and
adjusts the number of rows to serialize per batch based on this limit.
Setting a non-positive limit is grounds for short circuiting the scan.

Note: Use of this functionality by the client will come in a later
patch.

Change-Id: I3ca5ddff09f4910d12f80259bd11e4027f99aa7c
---
M src/kudu/common/scan_spec.cc
M src/kudu/common/scan_spec.h
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.h
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver.proto
10 files changed, 238 insertions(+), 49 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ca5ddff09f4910d12f80259bd11e4027f99aa7c
Gerrit-Change-Number: 9783
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-16 pt 2: add client-side limits on scanners

[Andrew Wong (Code Review)]

Hello David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: KUDU-16 pt 2: add client-side limits on scanners
..

KUDU-16 pt 2: add client-side limits on scanners

This patch adds a public API to allow the specification of
per-client-side-scanner limits on the number of rows returned. Each
scanner will maintain a count of the number of rows already read, and
adjust the server-side limit upon sending the next scan request.

A couple of unit tests are included to verify that the limits act as
expected. I also verified that lowering the limit reduces the number of
bytes read on disk (at the granularity of a single scan batch at a
time).

Note: this patch does not implement the behavior for scan tokens.

Change-Id: Ib2d40e3d14e36f3bf1d09a4bfdb3e17a745d244d
---
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/scanner-internal.cc
M src/kudu/client/scanner-internal.h
7 files changed, 188 insertions(+), 40 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib2d40e3d14e36f3bf1d09a4bfdb3e17a745d244d
Gerrit-Change-Number: 9790
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-16 pt 1: add server-side limits on scanners

[Andrew Wong (Code Review)]

Hello Tidy Bot, Dan Burkert, David Ribeiro Alves, Kudu Jenkins, Adar Dembo, 
Grant Henke,

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

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

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

Change subject: KUDU-16 pt 1: add server-side limits on scanners
..

KUDU-16 pt 1: add server-side limits on scanners

This patch adds the functionality to specify limits to the number of
rows returned per tablet server scanner. Internally, each server-side
scanner now tracks the number of rows it has already returned and
adjusts the number of rows to serialize per batch based on this limit.
Setting a non-positive limit is grounds for short circuiting the scan.

Note: Use of this functionality by the client will come in a later
patch.

Change-Id: I3ca5ddff09f4910d12f80259bd11e4027f99aa7c
---
M src/kudu/common/scan_spec.cc
M src/kudu/common/scan_spec.h
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.h
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver.proto
10 files changed, 238 insertions(+), 49 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3ca5ddff09f4910d12f80259bd11e4027f99aa7c
Gerrit-Change-Number: 9783
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-2290: Tool to re-create a tablet

[Will Berkeley (Code Review)]

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

Change subject: KUDU-2290: Tool to re-create a tablet
..


Patch Set 7:

> Build Failed

Unrelated, pre-existing race. See KUDU-2058.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c
Gerrit-Change-Number: 9393
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 21:45:28 +
Gerrit-HasComments: No

[kudu-CR] [examples] Improve loadgen example

[Will Berkeley (Code Review)]

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

Change subject: [examples] Improve loadgen example
..


Patch Set 1: Verified+1

This code doesn't affect tests so overriding Jenkins.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc0e655f56adcedec2c3dd8a8b3c900e3b2e7803
Gerrit-Change-Number: 9853
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 21:40:15 +
Gerrit-HasComments: No

[kudu-CR] [examples] Improve loadgen example

[Will Berkeley (Code Review)]

Will Berkeley has removed a vote on this change.

Change subject: [examples] Improve loadgen example
..


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ibc0e655f56adcedec2c3dd8a8b3c900e3b2e7803
Gerrit-Change-Number: 9853
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [tools] Add a tool to recover master data from tablet servers

[Will Berkeley (Code Review)]

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

Change subject: [tools] Add a tool to recover master data from tablet servers
..


Patch Set 4:

Quick update on this: I'm planning on writing up a quick comparison between 
different methods so we can discuss them and figure out the best way forward.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If29e421d466a531ebad72e281ae27e74e458f8c6
Gerrit-Change-Number: 9490
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 21:38:29 +
Gerrit-HasComments: No

[kudu-CR] [examples] Add licenses to Python examples

[Will Berkeley (Code Review)]

Will Berkeley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9858


Change subject: [examples] Add licenses to Python examples
..

[examples] Add licenses to Python examples

This also adds a README to the graphite example warning that it's
not a real integration, just an example.

Change-Id: I8825bfac0416fc19fdb99c972eb104898ba78e7d
---
M examples/python/dstat-kudu/README.md
M examples/python/dstat-kudu/kudu_dstat.py
A examples/python/graphite-kudu/README.adoc
M examples/python/graphite-kudu/kudu/__init__.py
M examples/python/graphite-kudu/kudu/kudu_graphite.py
M examples/python/graphite-kudu/setup.py
M python/setup.cfg
7 files changed, 108 insertions(+), 3 deletions(-)



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

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

[kudu-CR] rpc-test: fix multi-threaded test from running too long

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9856 )

Change subject: rpc-test: fix multi-threaded test from running too long
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I827a07d5b1d2463cb845ef6b04ec36036b2ddfbb
Gerrit-Change-Number: 9856
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 29 Mar 2018 21:27:37 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2384. Don't collect thread stacks when running under debugger

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9835 )

Change subject: KUDU-2384. Don't collect thread stacks when running under 
debugger
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib17ffb9514e9b440b8d97843374d2a60913a398b
Gerrit-Change-Number: 9835
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 29 Mar 2018 21:26:32 +
Gerrit-HasComments: No

[kudu-CR] rpc-test: fix multi-threaded test from running too long

[Sailesh Mukil (Code Review)]

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

Change subject: rpc-test: fix multi-threaded test from running too long
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I827a07d5b1d2463cb845ef6b04ec36036b2ddfbb
Gerrit-Change-Number: 9856
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 29 Mar 2018 21:14:45 +
Gerrit-HasComments: No

[kudu-CR] rpc-test: fix multi-threaded test from running too long

[Todd Lipcon (Code Review)]

Hello Sailesh Mukil,

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

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

to review the following change.


Change subject: rpc-test: fix multi-threaded test from running too long
..

rpc-test: fix multi-threaded test from running too long

In TSAN builds this test was taking several minutes. This makes the test
run based on a timer rather than an explicit count.

Change-Id: I827a07d5b1d2463cb845ef6b04ec36036b2ddfbb
---
M src/kudu/rpc/rpc-test.cc
1 file changed, 6 insertions(+), 2 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I827a07d5b1d2463cb845ef6b04ec36036b2ddfbb
Gerrit-Change-Number: 9856
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Sailesh Mukil 

[kudu-CR] KUDU-2384. Don't collect thread stacks when running under debugger

[Todd Lipcon (Code Review)]

Hello Tidy Bot, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: KUDU-2384. Don't collect thread stacks when running under 
debugger
..

KUDU-2384. Don't collect thread stacks when running under debugger

This changes the stack watchdog and the 'collect all stacks' utility
code to not run when it appears that the process is running under a
tracer such as gdb or strace. This makes debugging Kudu processes
significantly easier because you don't need to know the magical
incantation 'handle SIGUSR2 nostop noprint'.

I tested manually by running a master with
--diagnostics-log-stack-traces-interval-ms=10 and attaching to it with
gdb. With this patch, I didn't see any signals sent.

Change-Id: Ib17ffb9514e9b440b8d97843374d2a60913a398b
---
M src/kudu/util/debug-util.cc
M src/kudu/util/debug-util.h
M src/kudu/util/kernel_stack_watchdog.cc
M src/kudu/util/os-util.cc
M src/kudu/util/os-util.h
5 files changed, 68 insertions(+), 4 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib17ffb9514e9b440b8d97843374d2a60913a398b
Gerrit-Change-Number: 9835
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] meta cache: improve multi-threaded scalability

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9760 )

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 29 Mar 2018 20:36:26 +
Gerrit-HasComments: No

[kudu-CR](branch-1.7.x) KUDU-2364 Add extra check in ksck for tserver ID

[Attila Bukor (Code Review)]

Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9855 )

Change subject: KUDU-2364 Add extra check in ksck for tserver ID
..


Patch Set 1:

> Patch Set 1: Verified-1
>
> Build Failed
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/12754/ : FAILURE

LINT failed for some reason but I don't see why.


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.7.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c18ba7af8eaa6f5e4d7842f18754d2c1e32526
Gerrit-Change-Number: 9855
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 17:21:53 +
Gerrit-HasComments: No

[kudu-CR] make shared: fix build for newer libc++

[Todd Lipcon (Code Review)]

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

Change subject: make_shared: fix build for newer libc++
..

make_shared: fix build for newer libc++

Previously, we used friendship with various internal classes in the 'std'
namespace to allow make_shared to work against classes with private
constructors. With the version of libc++ that comes with LLVM 6, these
tricks no longer work and I was unable to find a suitable friend definition.

This patch switches to using a different approach based on constructing
a locally-scoped subclass of the target class.

I also noticed that TypeEncodingInfo and TypeInfo were only using shared_ptr
due to the pre-C++11 prohibition of scoped_ptrs in containers, so switched
them to unique_ptr.

Change-Id: Ib0dd0338ee531ab3578ba7291637860b56ba6230
Reviewed-on: http://gerrit.cloudera.org:8080/9847
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/cfile/type_encodings.cc
M src/kudu/cfile/type_encodings.h
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_reader.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/master/ts_descriptor.cc
M src/kudu/master/ts_descriptor.h
M src/kudu/rpc/periodic.cc
M src/kudu/rpc/periodic.h
M src/kudu/util/make_shared.h
13 files changed, 80 insertions(+), 84 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0dd0338ee531ab3578ba7291637860b56ba6230
Gerrit-Change-Number: 9847
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2303: Add ignoreNull option to upsertRows

[Todd Lipcon (Code Review)]

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

Change subject: KUDU-2303: Add ignoreNull option to upsertRows
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9834/2//COMMIT_MSG@7
PS2, Line 7: ignoreNull
Curious to hear others' opinions, but to me a name like 'treatNullAsUnset' or 
'nullIsUnset' is more clear than 'ignoreNull'. Thoughts?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide908ea29f572849eca0ba850ee197c1b22a07c8
Gerrit-Change-Number: 9834
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 16:56:28 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2303: Add ignoreNull option to upsertRows

[Will Berkeley (Code Review)]

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

Change subject: KUDU-2303: Add ignoreNull option to upsertRows
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9834/2//COMMIT_MSG@7
PS2, Line 7: KUDU-2303
I think you mean 2250?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide908ea29f572849eca0ba850ee197c1b22a07c8
Gerrit-Change-Number: 9834
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 16:48:19 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1977. Avoid extra reference counting for fast-path MetaCache lookups

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: KUDU-1977. Avoid extra reference counting for fast-path 
MetaCache lookups
..

KUDU-1977. Avoid extra reference counting for fast-path MetaCache lookups

This does a bit of refactoring of MetaCache so that fast-path lookups
can occur without allocating a LookupRpc object. This avoids an
allocation/deallocation pair as well as reference counting overhead on
various objects. The reference counting was seen as a bottleneck in a
workload like:

$ kudu perf loadgen -num-threads 16 \
  -num-rows-per-thread $[10*1000*1000] -table-num-replicas 3 \
  -table-num-buckets=100 

I ran before/after on the above command writing into a large (100+-node
cluster) that I had handy. A few key stats:

Before:

  time total  : 90676.3 ms
1458961.567442  task-clock (msec) #   15.954 CPUs utilized

  Top lines of profile:

27.21%  kudu kudu [.] 
kudu::client::internal::MetaCache::LookupTabletByKeyFastPath
14.95%  kudu kudu [.] 
kudu::subtle::RefCountedThreadSafeBase::AddRef
12.52%  kudu kudu [.] 
kudu::subtle::RefCountedThreadSafeBase::Release
 4.75%  kudu kudu [.] 
kudu::client::internal::MetaCache::LookupTabletByKey
 3.04%  kudu kudu [.] operator new[]
 1.78%  rpc reactor-387  kudu [.] 
std::unordered_set::erase

After:

  time total  : 60796.6 ms
 693975.355549  task-clock (msec) #   11.154 CPUs utilized
  Top lines of profile:
14.05%  kudu kudu   [.] 
kudu::client::internal::MetaCache::LookupTabletByKeyFastPath
 7.89%  kudu kudu   [.] operator new[]
 4.96%  rpc reactor-397  kudu   [.] 
std::unordered_set::erase
 4.47%  kudu libc-2.17.so   [.] __memcpy_ssse3_back
 3.42%  rpc reactor-397  kudu   [.] 
kudu::client::KuduInsert::~KuduInsert

In other words, this reduced CPU by more than 2x and throughput by about 50%.
Results on the above were relatively stable across multiple runs, with
wall-clock showing more variance than CPU-time.

The new bottleneck seems to be the rw_spinlock in MetaCache.

Change-Id: I5f17ffa88289c766b5b168b22da9781bf78f5592
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-test.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/client/partitioner-internal.cc
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/schema.h
8 files changed, 140 insertions(+), 98 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f17ffa88289c766b5b168b22da9781bf78f5592
Gerrit-Change-Number: 9753
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: meta_cache: improve multi-threaded scalability
..

meta_cache: improve multi-threaded scalability

When profiling a multi-threaded 'kudu perf loadgen' I found the
bottleneck was on the rw_spinlock acquisition in
MetaCache::LookupTabletByKeyFastPath as well as some contention in
RemoteTablet::stale().

This fixes the first by switching to a percpu_rwlock and the second by
using an atomic bool instead of a spinlock-protected bool.

This moved LookupTabletByKeyFastPath from the first line of the profile
(14% of CPU) down to the 8th line (3% of CPU).

Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
---
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/util/locks.h
3 files changed, 25 insertions(+), 17 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] meta cache: improve multi-threaded scalability

[Todd Lipcon (Code Review)]

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

Change subject: meta_cache: improve multi-threaded scalability
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h
File src/kudu/client/meta_cache.h:

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/client/meta_cache.h@261
PS1, Line 261:   std::atomic stale_;
> Previous discussion: https://gerrit.cloudera.org/c/1842/
Only thing i really dislike about std::atomic is that the API allows for 
Release_Load and Acquire_Store as follows:

std::atomic x;
int v = x.load(std::memory_order_release);
x.store(v, std::memory_order_acquire);

... but it turns out that the C++ memory model prohibits such things from 
existing. Rather than crash or give a warning or somesuch, it silently allows 
them without emitting the memory barriers required by x86 (which are emitted by 
our own Release_Load and Acquire_Store macros). Granted, those two operations 
are somewhat more esoteric, but they're useful for various low-level things 
like seq-locks. It appears we only use them in a handful of places in Kudu and 
some of them are potentially not even correct usage :)

Anyway, I feel like we should probably move to std::atomic for most use cases, 
and perhaps see if we can write a clang-tidy checker for the above incorrect 
usages and contribute it upstream.


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

http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@142
PS1, Line 142: // Usage:
 : //   percpu_rwlock mylock;
 : //
 : //   // Lock shared:
 : //   {
 : // boost::shared_lock lock(mylock.get_lock());
 : // ...
 : //   }
 : //
 : //   // Lock exclusive:
 : //
 : //   {
 : // boost::lock_guard lock(mylock);
 : // ...
 : //   }
> Mind updating this with the lock guards we actually use now?
Done


http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@142
PS1, Line 142: // Usage:
 : //   percpu_rwlock mylock;
 : //
 : //   // Lock shared:
 : //   {
 : // boost::shared_lock lock(mylock.get_lock());
 : // ...
 : //   }
 : //
 : //   // Lock exclusive:
 : //
 : //   {
 : // boost::lock_guard lock(mylock);
 : // ...
 : //   }
> I think you missed this one.
sorry, my network crapped out mid-push and apparently I pushed an old version. 
take 2...


http://gerrit.cloudera.org:8080/#/c/9760/1/src/kudu/util/locks.h@239
PS1, Line 239: rw_spinlock lock;
> Gotcha. Could you add a blurb about this optimization to the class comment?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38c8a94ea177bfa4f4e2048355464f76a5daa2ba
Gerrit-Change-Number: 9760
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 29 Mar 2018 16:42:14 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2290: Tool to re-create a tablet

[Will Berkeley (Code Review)]

Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: KUDU-2290: Tool to re-create a tablet
..

KUDU-2290: Tool to re-create a tablet

This adds a tool `kudu tablet unsafe_replace_tablet` that can be
used to replace a tablet with another having the same partition
information. This tool is useful when a tablet has permanently
lost all replicas because it can repair the tablet's table.
Before, the table would have to be recreated to recover from a
permanently lost tablet.

Due to KUDU-2376, the new test in replace_tablet-itest fails. Since
this tool is meant to fix broken tables, I think it's still valuable
to add this tool in the meantime.

Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c
---
M src/kudu/client/client-internal.cc
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/master-stress-test.cc
A src/kudu/integration-tests/replace_tablet-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_tablet.cc
14 files changed, 506 insertions(+), 9 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c
Gerrit-Change-Number: 9393
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2290: Tool to re-create a tablet

[Will Berkeley (Code Review)]

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

Change subject: KUDU-2290: Tool to re-create a tablet
..


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9393/6/src/kudu/integration-tests/replace_tablet-itest.cc
File src/kudu/integration-tests/replace_tablet-itest.cc:

http://gerrit.cloudera.org:8080/#/c/9393/6/src/kudu/integration-tests/replace_tablet-itest.cc@55
PS6, Line 55: ExternalMiniClusterITestBase(),
> warning: initializer for base class 'kudu::ExternalMiniClusterITestBase' is
Done


http://gerrit.cloudera.org:8080/#/c/9393/6/src/kudu/integration-tests/replace_tablet-itest.cc@121
PS6, Line 121:   while (workload.rows_inserted() < 2 * kNumRows) {
> warning: either cast from 'int' to 'long' is ineffective, or there is loss
Done


http://gerrit.cloudera.org:8080/#/c/9393/6/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:

http://gerrit.cloudera.org:8080/#/c/9393/6/src/kudu/tools/tool_action_common.cc@554
PS6, Line 554: LeaderMasterProxy::LeaderMasterProxy(const 
client::sp::shared_ptr& client) :
> warning: pass by value and use std::move [modernize-pass-by-value]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c
Gerrit-Change-Number: 9393
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 16:38:34 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2303: Add ignoreNull option to upsertRows

[Dan Burkert (Code Review)]

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

Change subject: KUDU-2303: Add ignoreNull option to upsertRows
..


Patch Set 2:

(1 comment)

> Patch Set 2:
>
> (5 comments)

http://gerrit.cloudera.org:8080/#/c/9834/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/OperationType.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/OperationType.scala:

http://gerrit.cloudera.org:8080/#/c/9834/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/OperationType.scala@43
PS2, Line 43: private[kudu] case object UpsertIgnoreNull extends OperationType {
> If we keep adding options to modify the behavior of these operations we're
On second thought it may be cleaner to just introduce a WriteOptions class and 
allow extensibility via adding fields.  This could be passed to the 
insert/upsert/etc calls in KuduContext.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide908ea29f572849eca0ba850ee197c1b22a07c8
Gerrit-Change-Number: 9834
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 14:45:11 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2290: Tool to re-create a tablet

[Will Berkeley (Code Review)]

Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: KUDU-2290: Tool to re-create a tablet
..

KUDU-2290: Tool to re-create a tablet

This adds a tool `kudu tablet unsafe_replace_tablet` that can be
used to replace a tablet with another having the same partition
information. This tool is useful when a tablet has permanently
lost all replicas because it can repair the tablet's table.
Before, the table would have to be recreated to recover from a
permanently lost tablet.

Due to KUDU-2376, the new test in replace_tablet-itest fails. Since
this tool is meant to fix broken tables, I think it's still valuable
to add this tool in the meantime.

Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c
---
M src/kudu/client/client-internal.cc
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/master-stress-test.cc
A src/kudu/integration-tests/replace_tablet-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/master/master_service.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_tablet.cc
14 files changed, 502 insertions(+), 9 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifbefbde68e3ca724f04efe0426a3906e5c33ed3c
Gerrit-Change-Number: 9393
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2364 Add extra check in ksck for tserver ID

[Will Berkeley (Code Review)]

Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9787 )

Change subject: KUDU-2364 Add extra check in ksck for tserver ID
..

KUDU-2364 Add extra check in ksck for tserver ID

ksck did not validate tablet server ID when checking connectivity.
Whenever the TabletServer was nuked and readded, ksck would report the
connection was successful to the old tablet servers.

Now it will report an error like below:

./bin/kudu cluster ksck localhost
Connected to the Master
WARNING: Unable to connect to Tablet Server d92197fa2f034b33aa0bf998c41f637b
(va1022.halxg.cloudera.com:7073): Remote error: ID reported by tablet server
(50effcf1fe284ab693e7d1d43c5f18ad) doesn't match the expected ID:
d92197fa2f034b33aa0bf998c41f637b
Tablet Server Summary
   UUID   |  RPC Address   |  Status
--++---
 22ec2c07d8aa4e1ba33a4eb42d4c3a21 | va1022.halxg.cloudera.com:7072 | HEALTHY
 50effcf1fe284ab693e7d1d43c5f18ad | va1022.halxg.cloudera.com:7073 | HEALTHY
 a05c93549cca4ceebc275651d8117065 | va1022.halxg.cloudera.com:7074 | HEALTHY
 d92197fa2f034b33aa0bf998c41f637b | va1022.halxg.cloudera.com:7073 | 
WRONG_SERVER_UUID
WARNING: Fetched info from 3 Tablet Servers, 1 weren't reachable
The cluster doesn't have any matching tables
==
Errors:
==
error fetching info from tablet servers: Could not gather complete information 
from all
tablet servers

FAILED
Runtime error: ksck discovered errors

Change-Id: Ia2c18ba7af8eaa6f5e4d7842f18754d2c1e32526
Reviewed-on: http://gerrit.cloudera.org:8080/9787
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley 
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
5 files changed, 138 insertions(+), 4 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia2c18ba7af8eaa6f5e4d7842f18754d2c1e32526
Gerrit-Change-Number: 9787
Gerrit-PatchSet: 15
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2303: Add ignoreNull option to upsertRows

[Will Berkeley (Code Review)]

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

Change subject: KUDU-2303: Add ignoreNull option to upsertRows
..


Patch Set 2:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9834/2//COMMIT_MSG@9
PS2, Line 9: upsert only non-Null
nit: Try to wrap commit message lines at about 80 chars.


http://gerrit.cloudera.org:8080/#/c/9834/2//COMMIT_MSG@10
PS2, Line 10:
nit:  Add "the".


http://gerrit.cloudera.org:8080/#/c/9834/2//COMMIT_MSG@12
PS2, Line 12: This
nit: "For example, this feature is useful..."


http://gerrit.cloudera.org:8080/#/c/9834/2//COMMIT_MSG@13
PS2, Line 13: where
nit: s/where/because



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide908ea29f572849eca0ba850ee197c1b22a07c8
Gerrit-Change-Number: 9834
Gerrit-PatchSet: 2
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 29 Mar 2018 08:14:54 +
Gerrit-HasComments: Yes