[kudu-CR] KUDU-1776: Fix "kudu remote replica copy" connecting to wildcard address

2016-12-08 Thread Dinesh Bhat (Code Review) 
Hello Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: KUDU-1776: Fix "kudu remote_replica copy" connecting to 
wildcard address
..

KUDU-1776: Fix "kudu remote_replica copy" connecting to wildcard address

"remote_replica copy" asks the source server for its bound address,
but that address could be wildcard (0.0.0.0) in a real cluster,
and if same address is stuffed in RPC, it fails the validation
at the source tablet server:

tablet_id: "834e7673f32e4802bdf26f9fff9162ef"
copy_peer_uuid: "893679e1b1dd4342bf1fb8f058ec2789"
copy_peer_addr {
host: "0.0.0.0"
port: 7050
}
UNKNOWN_ERROR: Invalid wildcard address to tablet copy from: 0.0.0.0 (resolved 
to 0.0.0.0)

Change-Id: Ie5d0a37b39a3774caab5b5d8dba3d9750bf5f21f
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/log-rolling-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
5 files changed, 97 insertions(+), 41 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie5d0a37b39a3774caab5b5d8dba3d9750bf5f21f
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Change version in master to 1.3.0-SNAPSHOT

2016-12-08 Thread Todd Lipcon (Code Review) 
Hello Jean-Daniel Cryans,

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

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

to review the following change.

Change subject: Change version in master to 1.3.0-SNAPSHOT
..

Change version in master to 1.3.0-SNAPSHOT

Change-Id: Ie52c36910dc0c333c1b3e86caa08edef2f45ad5e
---
M java/interface-annotations/pom.xml
M java/kudu-client-tools/pom.xml
M java/kudu-client/pom.xml
M java/kudu-csd/pom.xml
M java/kudu-csd/src/descriptor/service.sdl
M java/kudu-flume-sink/pom.xml
M java/kudu-mapreduce/pom.xml
M java/kudu-spark/pom.xml
M java/pom.xml
M python/setup.py
M version.txt
11 files changed, 12 insertions(+), 12 deletions(-)


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

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

[kudu-CR] Modify the default value of log dir flag.

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: Modify the default value of log_dir flag.
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icdc6539907e6ba105bf08b0ee349dbeb4089e08d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Kousuke Saruta 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] csd: two minor changes

2016-12-08 Thread Anonymous Coward (Code Review) 
Anonymous Coward #265 has posted comments on this change.

Change subject: csd: two minor changes
..


Patch Set 1:

(1 comment)

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

Line 23: So this probably makes the conversion a non-starter, right?
You can't share (template) names between service-level and role-level 
parameters, so this won't work, unfortunately.

You might be able to get them to work if you give them distinct names but the 
same config name. If you're lucky, you'll get "last one wins" semantics and the 
role-level one will appear last when both are set. You can then treat the 
role-level one as an override, with an empty default.

You'd have to test that works though, and we should officially support that on 
the CM side if you want to go that route (ie add a unit test that does this).

The safest thing to do, a method that I know will work, is to wait until Kudu 
is shipped with CM, then we can write some upgrade code to migrate your configs 
from service to role level.

You could also keep this patch as-is (role-level only, no service-level) if 
you're willing to just release note that the old service-level values will be 
lost. Wouldn't be the wildest thing to release note for the first GA release.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74b2fcb69fde9e0dbb232dba78db53cc1543b489
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Anonymous Coward #265
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Modify the default value of log dir flag.

2016-12-08 Thread Kousuke Saruta (Code Review) 
Kousuke Saruta has uploaded a new change for review.

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

Change subject: Modify the default value of log_dir flag.
..

Modify the default value of log_dir flag.

The document about configuration says that the default value of
log_dir flag is "/var/log/kudu" but it's "/tmp" in fact.

Change-Id: Icdc6539907e6ba105bf08b0ee349dbeb4089e08d
---
M docs/configuration.adoc
1 file changed, 2 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icdc6539907e6ba105bf08b0ee349dbeb4089e08d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Kousuke Saruta

[kudu-CR] csd: two minor changes

2016-12-08 Thread Adar Dembo (Code Review) 
Adar Dembo has uploaded a new change for review.

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

Change subject: csd: two minor changes
..

csd: two minor changes

Revert the memory limit scaleFactor to the default value (1.0). 1.3 is
recommended for Java processes; not sure why Kudu was using that.

Convert the core dump directory to two separate role-level parameters. I
relented and did this once I saw that Impala also defines its core dump
directory in this way. In testing, the conversion was fine when the
service-level parameter wasn't set. When it was set, I saw that the
overridden value carried over to the roles even after the conversion
(good!). However, attempting to remove the so-called "role" overrides in the
UI yielded an error (bad!).

  
/api/v8/clusters/Cluster%201/services/KUDU-1/roles/KUDU-KUDU_MASTER-1/config?message=Updated%20service%20and%20role%20type%20configurations.
 \
  { "message" : "Could not find config to delete with template name: 
core_dump_directory" }

So this probably makes the conversion a non-starter, right?

Change-Id: I74b2fcb69fde9e0dbb232dba78db53cc1543b489
---
M java/kudu-csd/src/descriptor/service.sdl
1 file changed, 19 insertions(+), 11 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I74b2fcb69fde9e0dbb232dba78db53cc1543b489
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

2016-12-08 Thread Alexey Serbin (Code Review) 
Alexey Serbin has submitted this change and it was merged.

Change subject: KUDU-1753 continue scan if tablet is being deleted
..


KUDU-1753 continue scan if tablet is being deleted

Updated TabletServiceImpl::HandleContinueScanRequest() to continue
serving in-progress scan requests even if the tablet was deleted
in the middle.

This patch also enables the TestDeleteTableWhileScanInProgress
integration test from the DeleteTableTest suite.

This is intended to fix the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Reviewed-on: http://gerrit.cloudera.org:8080/5346
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 30 insertions(+), 16 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy

[kudu-CR] Remove the clock from MvccManager

2016-12-08 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: Remove the clock from MvccManager
..


Patch Set 7:

> (6 comments)

oops, I got it wrong -- ignore that.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Remove the clock from MvccManager

2016-12-08 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: Remove the clock from MvccManager
..


Patch Set 7:

(6 comments)

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

PS6, Line 9: it's
nit: its


PS6, Line 11: it
nit: drop this


http://gerrit.cloudera.org:8080/#/c/5326/6/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:

PS6, Line 99: 
remove as well?


PS6, Line 440: 
Consider removing this as well.


http://gerrit.cloudera.org:8080/#/c/5326/7/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:

PS7, Line 99: 
Remove here and in other places in this test?


PS7, Line 440: 
Remove?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Remove the clock from MvccManager

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has submitted this change and it was merged.

Change subject: Remove the clock from MvccManager
..


Remove the clock from MvccManager

Now that safe time has it's own managing entity, there is no
need for MvccManager to have a reference to the clock since
it's no longer supposed to update it or wait on it.

Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
Reviewed-on: http://gerrit.cloudera.org:8080/5326
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet.cc
9 files changed, 56 insertions(+), 37 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Remove the clock from MvccManager

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: Remove the clock from MvccManager
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Make delete table-test less flaky

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: Make delete_table-test less flaky
..


Patch Set 1:

(1 comment)

how about 
TabletCopyClientSessionITest.TestStartTabletCopyWhileSourceBootstrapping ? it's 
also flaky in TSAN

http://gerrit.cloudera.org:8080/#/c/5421/1/src/kudu/integration-tests/delete_table-test.cc
File src/kudu/integration-tests/delete_table-test.cc:

PS1, Line 1199:  // Breaks here (found 2 !?!)
?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea7760009acc8a795225369721af97583974feba
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] log-rolling-itest: wait for log roller on server startup

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has submitted this change and it was merged.

Change subject: log-rolling-itest: wait for log roller on server startup
..


log-rolling-itest: wait for log roller on server startup

The log roller runs early in the startup sequence, but we were still
hitting cases where the test outran the roller.

Change-Id: Iefbf678163c0f4ba07eedfd762177c5ace5e35e6
Reviewed-on: http://gerrit.cloudera.org:8080/5431
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/log-rolling-itest.cc
1 file changed, 4 insertions(+), 1 deletion(-)

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



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

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

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

2016-12-08 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 continue scan if tablet is being deleted
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] KUDU-1757: add PartialRow.toString

2016-12-08 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1757: add PartialRow.toString
..


Patch Set 9: Code-Review+2

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

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

[kudu-CR] Fix linked list-test flakiness

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: Fix linked_list-test flakiness
..


Patch Set 1: Code-Review+2

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

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

[kudu-CR] Fix linked list-test flakiness

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has submitted this change and it was merged.

Change subject: Fix linked_list-test flakiness
..


Fix linked_list-test flakiness

A recent patch changed this test to do a snapshot scan at the end
which is now failing with a timeout. That patch looped this test
a bunch but, apparently, jenkins is even more stressful than
--stress_cpu_threads=4. I re-looped the test with --stress_cpu_theads=8
and observed 8/1000 failures.

The fix is to increase the timeout. After I did that I looped the
tests again and 1000/1000 passed.

Config:
KUDU_ALLOW_SLOW_TESTS=1 ../../build-support/dist_test.py loop -n 1000 \
bin/linked_list-test --stress_cpu_threads=8 --gtest_filter=*TestLoadAndVerify*

Results:
http://dist-test.cloudera.org//job?job_id=david.alves.1481225032.13214

Change-Id: Ice3d92e8112fc336d24600d6d817e37729ff8467
Reviewed-on: http://gerrit.cloudera.org:8080/5424
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Todd Lipcon 
---
M src/kudu/integration-tests/linked_list-test-util.h
1 file changed, 1 insertion(+), 1 deletion(-)

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



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

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

[kudu-CR] WIP Fix exactly once bug in tablet bootstrap

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: WIP Fix exactly_once bug in tablet bootstrap
..


Patch Set 1:

(1 comment)

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

Line 1186:   new_result->mutable_ops(i)->Clear();
do you need to first set up new_result->mutable_ops() to have the correct 
length?


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

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

[kudu-CR] Fix linked list-test flakiness

2016-12-08 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: Fix linked_list-test flakiness
..


Patch Set 1: Code-Review+2

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

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

[kudu-CR] [spark] KUDU-1641 Pushdown SparkSQL In predicates

2016-12-08 Thread Will Berkeley (Code Review) 
Will Berkeley has posted comments on this change.

Change subject: [spark] KUDU-1641 Pushdown SparkSQL In predicates
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5425/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala:

Line 208: assert(values.nonEmpty, "in list array must be nonempty")
> Do we need to check this?  Kudu should do the right thing and return an emp
No longer applicable. See below.


Line 209: values match {
> I think this block will be cleaner if you do the `values.map` part inside t
Actually the entire match block is unnecessary. The function can just create 
the in list predicate immediately.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia76dcbc0b5c5dcb2d28b468d8ed305b9f8eb8b28
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1524. Add a workaround for unflushable large cells

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1524. Add a workaround for unflushable large cells
..


KUDU-1524. Add a workaround for unflushable large cells

Previously, we had a hard-coded limit of 16MB for an individual cfile
block. This would cause a CHECK failure if someone inserted a cell
larger than this size.

We should probably limit large cells in the write path in a separate
patch, but it was also a bad idea to have this limit be a constant
instead of an 'unsafe' flag. This switches to using a flag for the value
so that, if we do end up in a situation like this, we can work around it
by bumping the flag instead of recompiling.

This also fixes the size limiting to be symmetric: we now always check
the size of the *uncompressed* block, which ensures that if we're able
to write a block, we will later be able to read it.

Change-Id: I245b52f2bc8b9d95716cacd340dca93f64846c73
Reviewed-on: http://gerrit.cloudera.org:8080/5282
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M src/kudu/cfile/block_compression.cc
M src/kudu/cfile/block_compression.h
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
4 files changed, 34 insertions(+), 27 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I245b52f2bc8b9d95716cacd340dca93f64846c73
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] log-rolling-itest: wait for log roller on server startup

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: log-rolling-itest: wait for log roller on server startup
..


Patch Set 1: Code-Review+2

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

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

[kudu-CR] [spark] KUDU-1641 Pushdown SparkSQL In predicates

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: [spark] KUDU-1641 Pushdown SparkSQL In predicates
..


Patch Set 2:

(1 comment)

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

Line 213: 
KuduPredicate.newInListPredicate(table.getSchema.getColumn(column), 
values.toList.asJava)
does Spark have any limit on the max number of elements in the IN list? do we? 
probably a good idea to put in some kind of limit here beyond which we don't 
try to push?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia76dcbc0b5c5dcb2d28b468d8ed305b9f8eb8b28
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] WIP: bug in exactly-once during tablet bootstrap

2016-12-08 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change.

Change subject: WIP: bug in exactly-once during tablet bootstrap
..


Patch Set 2:

oops sorry ended up rebasing your patch on master

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

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

[kudu-CR] WIP Fix exactly once bug in tablet bootstrap

2016-12-08 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has uploaded a new change for review.

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

Change subject: WIP Fix exactly_once bug in tablet bootstrap
..

WIP Fix exactly_once bug in tablet bootstrap

Change-Id: Ia33e6a937990c6a6f9d646cac300024ef95bcf7f
---
M src/kudu/tablet/tablet_bootstrap.cc
1 file changed, 30 insertions(+), 17 deletions(-)


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

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

[kudu-CR] WIP: bug in exactly-once during tablet bootstrap

2016-12-08 Thread David Ribeiro Alves (Code Review) 
Hello Kudu Jenkins,

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

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

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

Change subject: WIP: bug in exactly-once during tablet bootstrap
..

WIP: bug in exactly-once during tablet bootstrap

Here's a regression test for the bug which is causing
raft_consensus-itest to occasionally think it has inserted 23 rows when
in fact it has only inserted 20.

The issue is in the rewriting of logs during bootstrap. If we do a write
which gets a duplicate key error, the first time the COMMIT message is
written, it includes the error.

When the server restarts, it writes the COMMIT message again with only
'flushed: true' in the commit message. This is enough for bootstrap to
know not to bother to replay it on subsequent restarts, but it has lost
the error messages themselves.

If the server restarts again, at this point it doesn't rebuild a proper
response, but instead puts an errorless response into the ResultTracker.

So, if an operation hits an error, and then the tablet server restarts
twice while the client is still retrying, the client will falsely think
that its operation has succeeded.

This includes a regression test which shows the bug, but haven't looked
into fixing it yet.

Change-Id: I60b3b30b0705b4f9063b0d505cb9ab1ca24e470a
---
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 73 insertions(+), 0 deletions(-)


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

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

[kudu-CR] KUDU-1624. Fix data race in Trace::MetricsToJSON

2016-12-08 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1624. Fix data race in Trace::MetricsToJSON
..


Patch Set 1:

(1 comment)

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

Line 231:   e.second->MetricsToJSON(jw);
> We'll be making local copies of child_traces as we recurse, protected by th
I don't think it would be invalid, because for any given trace, we only 
snapshot its child_traces_ once.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If72ccf3fc8e0d727f2c40b978889597e89ad891a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1757: add PartialRow.toString

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1757: add PartialRow.toString
..


Patch Set 9:

(1 comment)

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

Line 1: // Licensed to the Apache Software Foundation (ASF) under one
> Add license.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Yanlong Zheng 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Yanlong Zheng 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1757: add PartialRow.toString

2016-12-08 Thread Dan Burkert (Code Review) 
Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: KUDU-1757: add PartialRow.toString
..

KUDU-1757: add PartialRow.toString

Operation.toString could fail if the operation's partial row had not had
its full primary key filled in yet. This adds a PartialRow.toString
method which can handle any unset columns, and changes
Operation.toString to use that. Also fixes a ByteBuffer usage bug in
PartialRow::appendCellValueDebugString

Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
3 files changed, 92 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Yanlong Zheng 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Yanlong Zheng

[kudu-CR] KUDU-1757: add PartialRow.toString

2016-12-08 Thread Dan Burkert (Code Review) 
Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: KUDU-1757: add PartialRow.toString
..

KUDU-1757: add PartialRow.toString

Operation.toString could fail if the operation's partial row had not had
its full primary key filled in yet. This adds a PartialRow.toString
method which can handle any unset columns, and changes
Operation.toString to use that. Also fixes a ByteBuffer usage bug in
PartialRow::appendCellValueDebugString

Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
3 files changed, 93 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Yanlong Zheng 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Yanlong Zheng

[kudu-CR] KUDU-1757: add PartialRow.toString

2016-12-08 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1757: add PartialRow.toString
..


Patch Set 7:

(1 comment)

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

Line 1: package org.apache.kudu.client;
Add license.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Yanlong Zheng 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Yanlong Zheng 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 [delete table-test] deleted-while-in-scan test

2016-12-08 Thread Alexey Serbin (Code Review) 
Hello Dinesh Bhat, Kudu Jenkins,

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

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

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

Change subject: KUDU-1753 [delete_table-test] deleted-while-in-scan test
..

KUDU-1753 [delete_table-test] deleted-while-in-scan test

Added an integration test to ensure a tablet server keeps all the
necessary data around until a scan operation ends even if tablet
is being deleted concurrently.

The test is disabled as it currently fails. A follow up patch will fix
the bug and enable the test.

This is in the context of the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: I983e68862b5535f2f95eedd41850a8a88e95e69c
---
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/integration-tests/external_mini_cluster-itest-base.cc
M src/kudu/integration-tests/external_mini_cluster-itest-base.h
3 files changed, 182 insertions(+), 32 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I983e68862b5535f2f95eedd41850a8a88e95e69c
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Adar Dembo (Code Review) 
Adar Dembo has submitted this change and it was merged.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


KUDU-1508: enforce block limit on lbm containers

This patch introduces the following machinery:
1. LBM containers can now be limited to a certain number of blocks using
   log_container_max_blocks. This is a soft limit; if the limit is reduced
   and the server restarted, existing containers may exceed it.
   The default value means "no limit except when vulnerable to KUDU-1508".
2. When we construct a block manager, we check whether it's an el6 kernel
   (that is, whether it's vulnerable to KUDU-1508).
3. At data directory opening time, we figure out whether it's vulnerable to
   KUDU-1508. If it is, and if it's running on an ext4 filesystem, we
   prescribe a particular block limit that depends on the filesystem's
   block size. These limits were arrived at via experimentation (see commit
   4923a74). They are likely more conservative than they need to be (they
   assume one extent per filesystem block with every other extent punched
   out), but with the addition of the file cache, the scalability overhead
   should be minimal.
4. The prescribed limit is passed into each container as it is instantiated.

I ran TestContainerWithManyHoles on several loopback ext4 filesystems on
el6.6. I tested block sizes of 1024, 2048, and 4096, each with and without
the fix. Without the fix, the filesystems needed to be repaired 100% of the
time. With the fix, they never needed repair.

Change-Id: I63576d2bf2cc61b2deb70f7e166f08e0d4c66543
Reviewed-on: http://gerrit.cloudera.org:8080/5403
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M src/kudu/experiments/KUDU-1508/run_test.sh
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
6 files changed, 322 insertions(+), 11 deletions(-)

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



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

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

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


Patch Set 7: Code-Review+2

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

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

[kudu-CR] KUDU-1757: add PartialRow.toString

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1757: add PartialRow.toString
..


Patch Set 6:

Hey Yanlong,

I tracked down the error to a bug I introduced recently.  I just pushed a new 
revision with the fix and a test.

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

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

[kudu-CR] KUDU-1757: add PartialRow.toString

2016-12-08 Thread Dan Burkert (Code Review) 
Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: KUDU-1757: add PartialRow.toString
..

KUDU-1757: add PartialRow.toString

Operation.toString could fail if the operation's partial row had not had
its full primary key filled in yet. This adds a PartialRow.toString
method which can handle any unset columns, and changes
Operation.toString to use that. Also fixes a ByteBuffer usage bug in
PartialRow::appendCellValueDebugString

Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
3 files changed, 76 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Yanlong Zheng 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Yanlong Zheng

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Adar Dembo (Code Review) 
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1508: enforce block limit on lbm containers
..

KUDU-1508: enforce block limit on lbm containers

This patch introduces the following machinery:
1. LBM containers can now be limited to a certain number of blocks using
   log_container_max_blocks. This is a soft limit; if the limit is reduced
   and the server restarted, existing containers may exceed it.
   The default value means "no limit except when vulnerable to KUDU-1508".
2. When we construct a block manager, we check whether it's an el6 kernel
   (that is, whether it's vulnerable to KUDU-1508).
3. At data directory opening time, we figure out whether it's vulnerable to
   KUDU-1508. If it is, and if it's running on an ext4 filesystem, we
   prescribe a particular block limit that depends on the filesystem's
   block size. These limits were arrived at via experimentation (see commit
   4923a74). They are likely more conservative than they need to be (they
   assume one extent per filesystem block with every other extent punched
   out), but with the addition of the file cache, the scalability overhead
   should be minimal.
4. The prescribed limit is passed into each container as it is instantiated.

I ran TestContainerWithManyHoles on several loopback ext4 filesystems on
el6.6. I tested block sizes of 1024, 2048, and 4096, each with and without
the fix. Without the fix, the filesystems needed to be repaired 100% of the
time. With the fix, they never needed repair.

Change-Id: I63576d2bf2cc61b2deb70f7e166f08e0d4c66543
---
M src/kudu/experiments/KUDU-1508/run_test.sh
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
6 files changed, 322 insertions(+), 11 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I63576d2bf2cc61b2deb70f7e166f08e0d4c66543
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1524. Add a workaround for unflushable large cells

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1524. Add a workaround for unflushable large cells
..


Patch Set 2: Code-Review+2

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

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

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


Patch Set 6:

Verified on macOS

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

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

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


Patch Set 6:

(1 comment)

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

Line 328:   int64_t total_blocks_written_ = 0;
Here and for the flag the # of blocks is int64_t, but for the limits you have 
changed it to int32_t.  Should it be consistent?


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

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

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


Patch Set 4:

(3 comments)

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

Line 1341: 
> Could you add a test that fills out a container to the block limit, and the
Done


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

Line 67:  "bug with hole punching on ext4.");
> Might help to include the JIRA #.
Done


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

Line 290:   static const std::map per_fs_block_size_block_limits;
> please use a fixed-size int type.
Done


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

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

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


Patch Set 4:

(3 comments)

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

Line 1341: 
Could you add a test that fills out a container to the block limit, and then 
restarts with a higher limit, and continues writing?  This is to simulate a 
user upgrading from a buggy kernel to a non-buggy kernel, and ensuring that the 
previously limited containers can be written to.


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

Line 67:  "bug with hole punching on ext4.");
Might help to include the JIRA #.


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

Line 290:   static const std::map per_fs_block_size_block_limits;
please use a fixed-size int type.


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

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

[kudu-CR] KUDU-1624. Fix data race in Trace::MetricsToJSON

2016-12-08 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: KUDU-1624. Fix data race in Trace::MetricsToJSON
..


Patch Set 1: Code-Review+2

(1 comment)

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

Line 231:   e.second->MetricsToJSON(jw);
We'll be making local copies of child_traces as we recurse, protected by the 
lock of course. Is it OK that the entirety of the MetricsToJSON() call operates 
on this somewhat inconsistent view of the trace tree? That is, new child traces 
may be added during the recursion; that doesn't make the JSON output invalid, 
does it?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If72ccf3fc8e0d727f2c40b978889597e89ad891a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Adar Dembo (Code Review) 
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1508: enforce block limit on lbm containers
..

KUDU-1508: enforce block limit on lbm containers

This patch introduces the following machinery:
1. LBM containers can now be limited to a certain number of blocks using
   log_container_max_blocks. This is a soft limit; if the limit is reduced
   and the server restarted, existing containers may exceed it.
   The default value means "no limit except when vulnerable to KUDU-1508".
2. When we construct a block manager, we check whether it's an el6 kernel
   (that is, whether it's vulnerable to KUDU-1508).
3. At data directory opening time, we figure out whether it's vulnerable to
   KUDU-1508. If it is, and if it's running on an ext4 filesystem, we
   prescribe a particular block limit that depends on the filesystem's
   block size. These limits were arrived at via experimentation (see commit
   4923a74). They are likely more conservative than they need to be (they
   assume one extent per filesystem block with every other extent punched
   out), but with the addition of the file cache, the scalability overhead
   should be minimal.
4. The prescribed limit is passed into each container as it is instantiated.

I ran TestContainerWithManyHoles on several loopback ext4 filesystems on
el6.6. I tested block sizes of 1024, 2048, and 4096, each with and without
the fix. Without the fix, the filesystems needed to be repaired 100% of the
time. With the fix, they never needed repair.

Change-Id: I63576d2bf2cc61b2deb70f7e166f08e0d4c66543
---
M src/kudu/experiments/KUDU-1508/run_test.sh
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
6 files changed, 311 insertions(+), 11 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I63576d2bf2cc61b2deb70f7e166f08e0d4c66543
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


Patch Set 4:

(5 comments)

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

Line 1247: TEST_F(LogBlockManagerTest, TestContainerWithManyHoles) {
> BTW - this disabling this test on OS X is a valid way forward, but that div
Ugh. I forgot to add RETURN_NOT_LOG_BLOCK_MANAGER() here and in the other test.

I'm not worried too much about the error; there's a 
RETURN_NOT_LOG_BLOCK_MANAGER() in SetUp() so this test is manipulating an 
instantiated but not Open()ed log block manager.


Line 1342: TEST_F(LogBlockManagerTest, TestContainerBlockLimiting) {
> Same here.
Done


http://gerrit.cloudera.org:8080/#/c/5403/4/src/kudu/util/env.h
File src/kudu/util/env.h:

Line 41: typedef std::unordered_map MountMap;
> this can be removed now
Done


Line 573: struct Mount {
> can be removed, right?
Done


http://gerrit.cloudera.org:8080/#/c/5403/4/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 20: #include 
> This isn't found on macOS.  Moving it into the non-mac block makes everythi
Done


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

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

[kudu-CR] Fix linked list-test flakiness

2016-12-08 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Fix linked_list-test flakiness
..

Fix linked_list-test flakiness

A recent patch changed this test to do a snapshot scan at the end
which is now failing with a timeout. That patch looped this test
a bunch but, apparently, jenkins is even more stressful than
--stress_cpu_threads=4. I re-looped the test with --stress_cpu_theads=8
and observed 8/1000 failures.

The fix is to increase the timeout. After I did that I looped the
tests again and 1000/1000 passed.

Config:
KUDU_ALLOW_SLOW_TESTS=1 ../../build-support/dist_test.py loop -n 1000 \
bin/linked_list-test --stress_cpu_threads=8 --gtest_filter=*TestLoadAndVerify*

Results:
http://dist-test.cloudera.org//job?job_id=david.alves.1481225032.13214

Change-Id: Ice3d92e8112fc336d24600d6d817e37729ff8467
---
M src/kudu/integration-tests/linked_list-test-util.h
1 file changed, 1 insertion(+), 1 deletion(-)


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

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

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


Patch Set 4:

(1 comment)

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

Line 1247: TEST_F(LogBlockManagerTest, TestContainerWithManyHoles) {
> This test fails on macOS: https://gist.github.com/6c13d83a0032a9168a37bb1be
BTW - this disabling this test on OS X is a valid way forward, but that divide 
by 0 should probably be checked.


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

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

[kudu-CR] KUDU-1508: enforce block limit on lbm containers

2016-12-08 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
..


Patch Set 4:

(3 comments)

Still doing a full review, but here are some notes from trying to run on macOS.

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

Line 1247: TEST_F(LogBlockManagerTest, TestContainerWithManyHoles) {
This test fails on macOS: 
https://gist.github.com/6c13d83a0032a9168a37bb1be4e1a010


Line 1342: TEST_F(LogBlockManagerTest, TestContainerBlockLimiting) {
Same here.


http://gerrit.cloudera.org:8080/#/c/5403/4/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 20: #include 
This isn't found on macOS.  Moving it into the non-mac block makes everything 
OK.


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

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

[kudu-CR] KUDU-1753 [delete table-test] deleted-while-in-scan test

2016-12-08 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 [delete_table-test] deleted-while-in-scan test
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5345/7/src/kudu/integration-tests/delete_table-test.cc
File src/kudu/integration-tests/delete_table-test.cc:

PS7, Line 1329:  // Once the first batch of data has been fetched and there is 
some more
  : // to fetch, delete the table. On the error, do not dump 
stacks: this
  : // assumes DeleteTable() works; if not, that's not the test 
which targeted
  : // to troubleshooting DeleteTable(), there are tests for 
that in this file.
  : NO_FATALS(DeleteTable(w.table_name(), 
ON_ERROR_DO_NOT_DUMP_STACKS));
> cool, just a matter of rephrasing/deleting the comment then
oh and I meant loop with the fix


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I983e68862b5535f2f95eedd41850a8a88e95e69c
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 [delete table-test] deleted-while-in-scan test

2016-12-08 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 [delete_table-test] deleted-while-in-scan test
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5345/7/src/kudu/integration-tests/delete_table-test.cc
File src/kudu/integration-tests/delete_table-test.cc:

PS7, Line 1329:  // Once the first batch of data has been fetched and there is 
some more
  : // to fetch, delete the table. On the error, do not dump 
stacks: this
  : // assumes DeleteTable() works; if not, that's not the test 
which targeted
  : // to troubleshooting DeleteTable(), there are tests for 
that in this file.
  : NO_FATALS(DeleteTable(w.table_name(), 
ON_ERROR_DO_NOT_DUMP_STACKS));
> I'm not saying errors are about to be ignored.  I'm saying that stacks woul
cool, just a matter of rephrasing/deleting the comment then


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I983e68862b5535f2f95eedd41850a8a88e95e69c
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 [delete table-test] deleted-while-in-scan test

2016-12-08 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1753 [delete_table-test] deleted-while-in-scan test
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5345/7/src/kudu/integration-tests/delete_table-test.cc
File src/kudu/integration-tests/delete_table-test.cc:

PS7, Line 1329:  // Once the first batch of data has been fetched and there is 
some more
  : // to fetch, delete the table. On the error, do not dump 
stacks: this
  : // assumes DeleteTable() works; if not, that's not the test 
which targeted
  : // to troubleshooting DeleteTable(), there are tests for 
that in this file.
  : NO_FATALS(DeleteTable(w.table_name(), 
ON_ERROR_DO_NOT_DUMP_STACKS));
> hum, this is weird. your saying that you are ignoring delete table errors b
I'm not saying errors are about to be ignored.  I'm saying that stacks would 
not be dumped in case of an error -- that's the comment I added by Dinesh's 
request.  But it seems the comment adds more confusion than clarity.  Probably, 
I should remove the comment.

There are couple of checks to make sure the table is deleted: check in the code 
below.  If you think it's necessary to add some other way to check the table is 
deleted -- please let me know.

Yes, I looped the test -- it fails 100% at ASSERT_TRUE(s.ok()) << s.ToString(); 
around at line 1348.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I983e68862b5535f2f95eedd41850a8a88e95e69c
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 [delete table-test] deleted-while-in-scan test

2016-12-08 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1753 [delete_table-test] deleted-while-in-scan test
..


Patch Set 7:

> (1 comment)
 > 
 > Consider +1 from my side, my browser is playing some tricks with me
 > not showing those radio buttons at the moment.

I see -- thanks for the explanation.  I'll remove using and add client::sp  
prefix there.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I983e68862b5535f2f95eedd41850a8a88e95e69c
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] Remove the clock from MvccManager

2016-12-08 Thread David Ribeiro Alves (Code Review) 
Hello Kudu Jenkins,

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

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

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

Change subject: Remove the clock from MvccManager
..

Remove the clock from MvccManager

Now that safe time has it's own managing entity, there is no
need for MvccManager to have a reference to the clock since
it's no longer supposed to update it or wait on it.

Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
---
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet.cc
9 files changed, 56 insertions(+), 37 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Remove the clock from MvccManager

2016-12-08 Thread David Ribeiro Alves (Code Review) 
Hello Kudu Jenkins,

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

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

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

Change subject: Remove the clock from MvccManager
..

Remove the clock from MvccManager

Now that safe time has it's own managing entity, there is no
need for MvccManager to have a reference to the clock since
it's no longer supposed to update it or wait on it.

Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet.cc
10 files changed, 58 insertions(+), 37 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b3359fb3e92193f37ebb36063790230a1cac7a8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

2016-12-08 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 continue scan if tablet is being deleted
..


Patch Set 6:

(2 comments)

lgtm, excepting nits. see my comment on the test patch though.

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

PS6, Line 1686: PREDICT_FALSE(!s.ok() && !s.IsIllegalState())
nit: you only need to check !s.IsIllegalState()


PS6, Line 1686: if (PREDICT_FALSE(!s.ok() && !s.IsIllegalState())) {
how about you make this more explicit:

if (PREDICT_FALSE(!s.ok() || tablet_ref_error_code != TABLET_NOT_RUNNING) ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 [delete table-test] deleted-while-in-scan test

2016-12-08 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 [delete_table-test] deleted-while-in-scan test
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5345/7/src/kudu/integration-tests/delete_table-test.cc
File src/kudu/integration-tests/delete_table-test.cc:

PS7, Line 1329:  // Once the first batch of data has been fetched and there is 
some more
  : // to fetch, delete the table. On the error, do not dump 
stacks: this
  : // assumes DeleteTable() works; if not, that's not the test 
which targeted
  : // to troubleshooting DeleteTable(), there are tests for 
that in this file.
  : NO_FATALS(DeleteTable(w.table_name(), 
ON_ERROR_DO_NOT_DUMP_STACKS));
hum, this is weird. your saying that you are ignoring delete table errors but 
then you're waiting forever for the table to be deleted. maybe this is just a 
matter of rephrasing the comment ? are you sure that the table is going to be 
deleted at this point?

did you loop this test a bunch?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I983e68862b5535f2f95eedd41850a8a88e95e69c
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1776: Fix "kudu remote replica copy" connecting to wildcard address

2016-12-08 Thread Dinesh Bhat (Code Review) 
Hello Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: KUDU-1776: Fix "kudu remote_replica copy" connecting to 
wildcard address
..

KUDU-1776: Fix "kudu remote_replica copy" connecting to wildcard address

"remote_replica copy" asks the source server for its bound address,
but that address could be wildcard (0.0.0.0) in a real cluster,
and if same address is stuffed in RPC, it fails the validation
at the source tablet server:

tablet_id: "834e7673f32e4802bdf26f9fff9162ef"
copy_peer_uuid: "893679e1b1dd4342bf1fb8f058ec2789"
copy_peer_addr {
host: "0.0.0.0"
port: 7050
}
UNKNOWN_ERROR: Invalid wildcard address to tablet copy from: 0.0.0.0 (resolved 
to 0.0.0.0)

Change-Id: Ie5d0a37b39a3774caab5b5d8dba3d9750bf5f21f
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
4 files changed, 98 insertions(+), 42 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie5d0a37b39a3774caab5b5d8dba3d9750bf5f21f
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] dist-test: allow collecting the tmp dir of failed tests

2016-12-08 Thread Mike Percy (Code Review) 
Mike Percy has submitted this change and it was merged.

Change subject: dist-test:  allow collecting the tmp dir of failed tests
..


dist-test:  allow collecting the tmp dir of failed tests

When debugging certain types of failures, especially with randomized
tests, it can be very useful to grab the TEST_TMPDIR of any failed
runs, since that includes the WAL, data files, etc, of any minicluster
servers used in the test.

This adds a --collect-tmpdir flag to our dist_test wrapper which causes
the dist test task to create a tarball of its test tmpdir if the task
fails. It puts the tarball in the log directory which is considered the
"artifact" of the task, so that the 'fetch' command downloads them.

Change-Id: I57d42d93e2459bafebe433ade3a9aa7a45eaeabe
Reviewed-on: http://gerrit.cloudera.org:8080/2336
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M build-support/dist_test.py
M build-support/run_dist_test.py
2 files changed, 19 insertions(+), 6 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I57d42d93e2459bafebe433ade3a9aa7a45eaeabe
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] Revert "KUDU-861 Support changing default, storage attributes"

2016-12-08 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: Revert "KUDU-861 Support changing default, storage attributes"
..


Revert "KUDU-861 Support changing default, storage attributes"

This reverts commit e16a541aedc171eadb738e969a48852ba9ed2909, new clients don't 
work
with old servers.

Change-Id: Ia15db5407af681a412763681bc4fd43e99968b0b
Reviewed-on: http://gerrit.cloudera.org:8080/5420
Reviewed-by: Will Berkeley 
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M src/kudu/cfile/rle_block.h
M src/kudu/client/client-test.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/value-internal.h
M src/kudu/client/value.cc
M src/kudu/common/common.proto
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.h
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
19 files changed, 117 insertions(+), 937 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia15db5407af681a412763681bc4fd43e99968b0b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley

[kudu-CR] Revert "KUDU-861 Support changing default, storage attributes"

2016-12-08 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has posted comments on this change.

Change subject: Revert "KUDU-861 Support changing default, storage attributes"
..


Patch Set 1:

Thanks Will, like we discussed on Slack let's put up this patch again when it 
has a solution for compatibility (using feature flags or something like that).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia15db5407af681a412763681bc4fd43e99968b0b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] Revert "KUDU-861 Support changing default, storage attributes"

2016-12-08 Thread Will Berkeley (Code Review) 
Will Berkeley has posted comments on this change.

Change subject: Revert "KUDU-861 Support changing default, storage attributes"
..


Patch Set 1: Code-Review+2

+2 pending Jenkins

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia15db5407af681a412763681bc4fd43e99968b0b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] Revert "KUDU-861 Support changing default, storage attributes"

2016-12-08 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has uploaded a new change for review.

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

Change subject: Revert "KUDU-861 Support changing default, storage attributes"
..

Revert "KUDU-861 Support changing default, storage attributes"

This reverts commit e16a541aedc171eadb738e969a48852ba9ed2909, new clients don't 
work
with old servers.

Change-Id: Ia15db5407af681a412763681bc4fd43e99968b0b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M src/kudu/cfile/rle_block.h
M src/kudu/client/client-test.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/value-internal.h
M src/kudu/client/value.cc
M src/kudu/common/common.proto
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.h
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
19 files changed, 117 insertions(+), 937 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia15db5407af681a412763681bc4fd43e99968b0b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans

[kudu-CR] KUDU-1796: Use last REPLICATE OpId instead of last COMMIT OpId to tombstone a tablet replica

2016-12-08 Thread Mike Percy (Code Review) 
Mike Percy has submitted this change and it was merged.

Change subject: KUDU-1796: Use last REPLICATE OpId instead of last COMMIT OpId 
to tombstone a tablet replica
..


KUDU-1796: Use last REPLICATE OpId instead of last COMMIT OpId to tombstone a 
tablet replica

The kudu-tool-test has been flaky because of the following failure:

kudu-tool-test.cc:1203: Failure
Value of: tombstoned_opid.index()
  Actual: 205
  Expected: last_logged_opid.index()
  Which is: 206

Issue here is, offline cli tool "local_replica delete" stored
last COMMIT OpId on the tablet superblock instead of last REPLICATE
OpId while tombstoning the tablet replica. Hence kudu-tool-test
ToolTest.TestLocalReplicaTombstoneDelete observed that sometimes
tombstoned_opid (COMMIT) lagged behind last_logged_opid (REPLICATE).

This patch fixes the tool to use the REPLICATE OpId from the WAL.
Testing: Ran 5000 iterations of current failing test.

Change-Id: I02e95f05fab80e94b1afd078b23e5e03eca6e42a
Reviewed-on: http://gerrit.cloudera.org:8080/5416
Reviewed-by: Mike Percy 
Tested-by: Kudu Jenkins
---
M src/kudu/tools/tool_action_local_replica.cc
1 file changed, 15 insertions(+), 15 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I02e95f05fab80e94b1afd078b23e5e03eca6e42a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1796: Use last REPLICATE OpId instead of last COMMIT OpId to tombstone a tablet replica

2016-12-08 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change.

Change subject: KUDU-1796: Use last REPLICATE OpId instead of last COMMIT OpId 
to tombstone a tablet replica
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02e95f05fab80e94b1afd078b23e5e03eca6e42a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1796: Use last REPLICATE OpId instead of last COMMIT OpId to tombstone a tablet replica

2016-12-08 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change.

Change subject: KUDU-1796: Use last REPLICATE OpId instead of last COMMIT OpId 
to tombstone a tablet replica
..


Patch Set 1:

(1 comment)

Whoops. Good catch. Looks good, just an error message nit.

http://gerrit.cloudera.org:8080/#/c/5416/1/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:

PS1, Line 192: "No Replicated OpIds found in the log"
How about: "No entries found in the write-ahead log"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02e95f05fab80e94b1afd078b23e5e03eca6e42a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1776: Fix "kudu remote replica copy" connecting to wildcard address

2016-12-08 Thread Dinesh Bhat (Code Review) 
Hello Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: KUDU-1776: Fix "kudu remote_replica copy" connecting to 
wildcard address
..

KUDU-1776: Fix "kudu remote_replica copy" connecting to wildcard address

"remote_replica copy" asks the source server for its bound address,
but that address could be wildcard (0.0.0.0) in a real cluster,
and if same address is stuffed in RPC, it fails the validation
at the source tablet server:

tablet_id: "834e7673f32e4802bdf26f9fff9162ef"
copy_peer_uuid: "893679e1b1dd4342bf1fb8f058ec2789"
copy_peer_addr {
host: "0.0.0.0"
port: 7050
}
UNKNOWN_ERROR: Invalid wildcard address to tablet copy from: 0.0.0.0 (resolved 
to 0.0.0.0)

Change-Id: Ie5d0a37b39a3774caab5b5d8dba3d9750bf5f21f
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
4 files changed, 90 insertions(+), 41 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie5d0a37b39a3774caab5b5d8dba3d9750bf5f21f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1757: fix appendCellValueDebugString, do not throw exception when a column is not set

2016-12-08 Thread Yanlong Zheng (Code Review) 
Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: KUDU-1757: fix appendCellValueDebugString, do not throw 
exception when a column is not set
..

KUDU-1757: fix appendCellValueDebugString, do not throw exception when a column 
is not set

Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
2 files changed, 37 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife05af5a11e95751b3cc259e786d36494aeb5d3a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Yanlong Zheng 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Yanlong Zheng