[kudu-CR] KUDU-1821. Noisy warning from catalog manager

[Jun He (Code Review)]

Jun He has uploaded a new change for review.

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

Change subject: KUDU-1821. Noisy warning from catalog manager
..

KUDU-1821. Noisy warning from catalog manager

This commit changes the returned status to `Uninitialized` if the
catalog manager is trying to start.
Add additional if statements for this case to show more details.

Change-Id: I6a9b8577e4d857c63df6e66c10d6cf20270cd41e
---
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/master-stress-test.cc
M src/kudu/master/catalog_manager.cc
3 files changed, 15 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6a9b8577e4d857c63df6e66c10d6cf20270cd41e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jun He 

[kudu-CR] [kudu-jepsen] added Jenkins script

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [kudu-jepsen] added Jenkins script
..

[kudu-jepsen] added Jenkins script

Added a shell script to build Kudu and run kudu-jepsen tests.

Currently, the set of nodes to run Kudu cluster should be provisioned
prior to running the test.  The test uses the Jenkins slave as the
Jepsen control node, running the control logic and the freshly built
Kudu Java client there.

All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH.  The private SSH key should be set prior
to running the test.

Restrictions:
  1. The Kudu cluster should consist of Linux machines of the same
 architecture and OS distro as the Jenkins slave.  This is because
 the test copies and runs the newly built Kudu server-side binaries
 and the 'kudu' CLI utility on corresponding Kudu cluster nodes.

  2. The kudu-jepsen test supports recent Debian distros only.
 This is because Jepsen lacks support for other Linux distros.

Change-Id: I36a7e890baabb5427b22daa3aeee58ed894b83d6
---
A src/kudu/scripts/jepsen.sh
1 file changed, 129 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I36a7e890baabb5427b22daa3aeee58ed894b83d6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [kudu-jepsen] Kudu Jepsen tests

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [kudu-jepsen] Kudu Jepsen tests
..


Patch Set 12:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj:

Line 1: (ns jepsen.kudu
> nit: need license headers on this and all other files in this commit
Done


Line 28:   (def flags ["--fs_wal_dir=/var/lib/kudu/master"
> eastwood lint says this is a "def-in-def" which is bad (since it makes 'fla
Done


PS12, Line 32:   (when (> (count (:masters test)) 1)
 : (conj flags (str "--master_addresses=" 
(concatenate-addresses (:masters test)
> confused by this - it seems like a no-op, since the result of the (when...)
Good catch -- yes, indeed.  That's a bug.  Fixed.


Line 46: (def ntp-common-opts ["statistics loopstats peerstats clockstats"
> could you 'slurp' these from a resource file instead?
Done


PS12, Line 64: masters
> I don't think NTP uses the term 'masters'. Probably better to say 'servers'
Done


Line 66:  [(str "server " (name (first masters))
> shouldn't this use all of the configured masters, not just the first? eg so
Done


PS12, Line 69: (defn db
 :   []
 :   "Apache Kudu."
> src/main/clojure/jepsen/kudu.clj:69:7: misplaced-docstrings: Possibly mispl
Done


PS12, Line 151: (into []
> https://github.com/bbatsov/clojure-style-guide says to use (vec ...) instea
Done


http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj:

Line 37:   ([name type] (.build (.key (new ColumnSchema$ColumnSchemaBuilder 
name, type) false)))
> can this be defined as (column-schema name type false) to avoid redundancy?
Good idea!  Done.


Line 38:   ([name type key?] (.build (.key (new 
ColumnSchema$ColumnSchemaBuilder name, type) key?
> how about:
Done


PS12, Line 51: columns (.getColumns (.getSchema row-result)
> again maybe the java programmer in me but I think (-> row-result .getSchema
Done


PS12, Line 55: case (.ordinal (.getType column))
 :   ;; Clojure transforms enums in literals
 :   ;; so we have to use ordinals :(
> what about using cond instead?
Good idea -- I replaced that with condp and it works.


PS12, Line 71: ->t
> the style guide I'm looking at says to use -> for "conversion functions" bu
Done: renamed into 'drain-scanner-into-tuples'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [kudu-jepsen] Kudu Jepsen tests

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [kudu-jepsen] Kudu Jepsen tests
..

[kudu-jepsen] Kudu Jepsen tests

This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
  * Extra nemeses for the read/write register linearizability test
  * Run multiple test scenarios in the scope of the register test
  * Starting up master server: wait for the catalog manager
  * Other unsorted fixes for more robust operation

The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.

The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin.  The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.

All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH.  The private SSH key should be set prior
to running the test:
  1. The public part of the SSH key should be added into the
 'authorized_keys' file for the root user on all cluster nodes.
  2. The private part of the SSH key should be provided to the test
 either by:
   * adding the key into the SSH agent on the control node
   * specifying the path to the key via 'sshKeyPath' property

Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:

  mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"

after bulding the top-level project with

  mvn compile -Pjepsen

Restrictions:
  1. The Kudu cluster should consist of Linux machines of the same
 architecture and OS distro as the machine where the Kudu
 kudu-master, kudu-tserver, and the 'kudu' CLI utility are built.

  2. As for now, the kudu-jepsen test runs on recent Debian distros
 only (jepsen supports only Debian Linux).

Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
13 files changed, 1,387 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KuduRDD.collect fails because of NoSerializableException

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KuduRDD.collect fails because of NoSerializableException
..


Patch Set 4:

I've put up a new gerrit based on Kousuke's suggestion: 
https://gerrit.cloudera.org/#/c/5636/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0463424481a33c66fd7464345c305062420cfe9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Kousuke Saruta 
Gerrit-Reviewer: Chris George 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kousuke Saruta 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1824. KuduRDD.collect fails because of NoSerializableException

[Dan Burkert (Code Review)]

Dan Burkert has uploaded a new patch set (#2).

Change subject: KUDU-1824. KuduRDD.collect fails because of 
NoSerializableException
..

KUDU-1824. KuduRDD.collect fails because of NoSerializableException

This also fixes a few style issues.

Change-Id: I42618188003d2eef66088f3101803d1750e4134b
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
M 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
A java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala
3 files changed, 45 insertions(+), 23 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I42618188003d2eef66088f3101803d1750e4134b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1824. KuduRDD.collect fails because of NoSerializableException

[Dan Burkert (Code Review)]

Hello Jean-Daniel Cryans, Todd Lipcon,

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

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

to review the following change.

Change subject: KUDU-1824. KuduRDD.collect fails because of 
NoSerializableException
..

KUDU-1824. KuduRDD.collect fails because of NoSerializableException

This also fixes a few style issues.

Change-Id: I42618188003d2eef66088f3101803d1750e4134b
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala
M 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
A java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala
3 files changed, 46 insertions(+), 23 deletions(-)


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

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

[kudu-CR] Initial draft of release notes and doc updates for 1.2

[Todd Lipcon (Code Review)]

Hello Dinesh Bhat, Kudu Jenkins,

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

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

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

Change subject: Initial draft of release notes and doc updates for 1.2
..

Initial draft of release notes and doc updates for 1.2

Change-Id: I08326171dd2bf6097a7594b95adca946bb5922eb
---
M docs/known_issues.adoc
M docs/release_notes.adoc
M docs/schema_design.adoc
3 files changed, 191 insertions(+), 13 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I08326171dd2bf6097a7594b95adca946bb5922eb
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Initial draft of release notes and doc updates for 1.2

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Initial draft of release notes and doc updates for 1.2
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5604/2/docs/release_notes.adoc
File docs/release_notes.adoc:

PS2, Line 66: an
> nit: a
Done


Line 214: [[rn_1.2.0_client_compatibility]
> Missing a ]
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08326171dd2bf6097a7594b95adca946bb5922eb
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Restructure release notes in preparation for 1.2 release

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Restructure release notes in preparation for 1.2 release
..


Restructure release notes in preparation for 1.2 release

* Moved the 1.1 release notes to the Prior Release Notes page
* On the Prior Release Notes page, removed the list of known
  limitations, upgrade instructions, compatibility notes, etc for each
  of the past releases. Those things aren't very useful in the case of
  the prior releases, and it would generally be better for people to
  refer to the documentation corresponding to that particular release if
  they are interested in those details.
* Moved the current version Known Issues and Limitations documentation
  to a new separate docs page.

Change-Id: Ia6684706ec9c0b774ec11805cab1d4a3f02412f0
Reviewed-on: http://gerrit.cloudera.org:8080/5602
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
A docs/known_issues.adoc
M docs/prior_release_notes.adoc
M docs/release_notes.adoc
M docs/support/jekyll-templates/document.html.erb
4 files changed, 235 insertions(+), 410 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia6684706ec9c0b774ec11805cab1d4a3f02412f0
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1741: Make MiniCluster and ExternalMiniCluster follow one semantic for Restart

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1741: Make MiniCluster and ExternalMiniCluster follow one 
semantic for Restart
..


Patch Set 1:

Intuitively restart means "stop then start" for me, and as shown by your patch 
there's definitely a need for a method that does both. IMO if you want to keep 
restart to mean "start a stopped process" then that's fine but we should also 
have something that stops then stars.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad33b7c46bfca3f277ccbca7d0420272f06a6633
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Restructure release notes in preparation for 1.2 release

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: Restructure release notes in preparation for 1.2 release
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6684706ec9c0b774ec11805cab1d4a3f02412f0
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Tightening ScanSpec primary bounds when range predicate exists

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged.

Change subject: Tightening ScanSpec primary bounds when range predicate exists
..


Tightening ScanSpec primary bounds when range predicate exists

Currently, push upper bound key predicates will break when meeting
a range predicate.
I think the upper bound key can be decreased.
For example, If PK = (a, b) and a < 2 && b < 3, PK can be further
tightning to PK < (1, 3).

Change-Id: I931a617605434700352d285fc2039033cf9eb07e
Reviewed-on: http://gerrit.cloudera.org:8080/5360
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M src/kudu/common/key_util-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/key_util.h
M src/kudu/common/scan_spec-test.cc
4 files changed, 221 insertions(+), 22 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I931a617605434700352d285fc2039033cf9eb07e
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Haijie Hong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] Tightening ScanSpec primary bounds when range predicate exists

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Tightening ScanSpec primary bounds when range predicate exists
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I931a617605434700352d285fc2039033cf9eb07e
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Haijie Hong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] Kudu Jepsen Tests - Initial Commit

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Kudu Jepsen Tests - Initial Commit
..


Patch Set 12:

> Hrm, it sounds like the subsequent patch rewrites/improves a lot of
 > what's in this one. Why not squash the patches so that reviewers
 > don't have to spend time looking at stuff that's already been
 > removed/changed?

OK, that sounds good to me -- I hope David does not mind.  Originally we have 
several patches because we were working on different things independently, but 
now we can merge that.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add some path / env related helper functions

[Dinesh Bhat (Code Review)]

Dinesh Bhat has posted comments on this change.

Change subject: Add some path / env related helper functions
..


Patch Set 3: Code-Review+1

(5 comments)

http://gerrit.cloudera.org:8080/#/c/5618/3/src/kudu/util/env_util-test.cc
File src/kudu/util/env_util-test.cc:

PS3, Line 79: We have to clean this up manually
If this path is part of test directory, do we still have to clean this up 
manually ? or are we cleaning it up only to verify Deleterecursive routine 
after CreateRecursive ?


PS3, Line 112: real_dir
Nit: symlink could have been created to test_dir_ itself instead of a new 
real_dir.


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

PS3, Line 206: Canonicalize
If we move this line to above L200 we don't need to special case symlink here. 
i.e, Canonicalize does more than just resolving symlink, so it's sufficient to 
check is_dir and continue at L203.


http://gerrit.cloudera.org:8080/#/c/5618/3/src/kudu/util/env_util.h
File src/kudu/util/env_util.h:

PS3, Line 74: .
Nit: IMO it would be good to add in the comment that this is emulating mkdir -p.


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

PS3, Line 56: SplitPath
Do we want to the name of the routine to be bit more intuitive ? Perhaps 
SplitPathIntoSegments (inline with JoinPathSegments) ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia664708a09493923abbf2ff4e56e3d49c62cf97e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1643 Prune hash partitions based on IN-list predicates

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KUDU-1643 Prune hash partitions based on IN-list predicates
..


Patch Set 12:

(9 comments)

Sorry this took so long, was out on holiday break for a stretch.  Will try to 
be much more responsive going forward.  Thanks again for taking this on!

http://gerrit.cloudera.org:8080/#/c/5176/12/src/kudu/common/partition_pruner-test.cc
File src/kudu/common/partition_pruner-test.cc:

Line 660:   // a in [0];
We automatically simplify single-valued IN list predicates to an equality 
predicate, so there is no need to test this case.  Could you change the 
remaining cases which use single value IN lists to use equality instead, just 
to make it more clear?


http://gerrit.cloudera.org:8080/#/c/5176/12/src/kudu/common/partition_pruner.cc
File src/kudu/common/partition_pruner.cc:

Line 154: // Search all combination of in-list and equality predicates.
Move this comment to the header.


PS12, Line 156: const PartitionSchema& partition_schema,
  : 
const PartitionSchema::HashBucketSchema& hash_bucket_schema,  // NOLINT(*)
  : 
const Schema& schema,
  : 
const ScanSpec& scan_spec
To avoid the NOLINT you can add a line break before the first argument and 
indent to 4 spaces, per the google style guide.


PS12, Line 165: FindOrNull
Use FindOrDie, since the predicate has already been checked, and the result 
isn't checked against nullptr.


PS12, Line 183: static_cast(col_offset)
this static cast can be avoided by making the col_offset a size_t to begin with.


PS12, Line 312: hash_bucket_bitset
I think you need to wrap hash_bucket_bitset in std::move in order to avoid a 
copy here.


http://gerrit.cloudera.org:8080/#/c/5176/12/src/kudu/common/partition_pruner.h
File src/kudu/common/partition_pruner.h:

PS12, Line 67: SearchHashOfColumnCombination
I think a better name for this method is 'PruneHashComponent', since it's being 
applied to just a single hash component.


PS12, Line 67: vector
use the fully qualified std::vector in header files.


Line 68:   const 
PartitionSchema::HashBucketSchema& hash_bucket_schema,  // NOLINT(*)
See comment .cc about wrapping and NOLINT.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I578ac3c5e6770b13b0bebe01127d7d8263aceb36
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Haijie Hong 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Haijie Hong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] Add Google Breakpad support to Kudu

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Add Google Breakpad support to Kudu
..


Patch Set 3:

(10 comments)

Does it work on OS X?  Consider adding corresponding ifdefs at least at the 
first pass to allow this to compile on OS X.

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

PS3, Line 21: #include 
nit: is it needed?


Line 21: #include 
nit: consider adding 


PS3, Line 22: #include 
nit: is it needed?


PS3, Line 24: #include "kudu/gutil/strings/util.h"
nit: is it needed?


PS3, Line 26: #include "kudu/util/logging.h"
nit: is it needed?


Line 28: #include "kudu/util/subprocess.h"
nit: consider including "kudu/util/monotime.h" (for MonoDelta)


http://gerrit.cloudera.org:8080/#/c/5620/3/src/kudu/master/master_main.cc
File src/kudu/master/master_main.cc:

PS3, Line 60: argv[0]
nit: consider adding a var for argv[0] and using it around instead.


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

PS3, Line 57: Set to empty to "
: "disable writing minidump files.
I see there is also 'enable_minidumps' flag which is the explicit control flag 
for this behavior.  From the usability point, I think it's better to have only 
one flag controlling minidumps generation, if possible.  Would be that a 
problem generating/writing minidumps when this property were effectively empty?


http://gerrit.cloudera.org:8080/#/c/5620/3/thirdparty/build-thirdparty.sh
File thirdparty/build-thirdparty.sh:

PS3, Line 284: F_ALL
What is F_ALL?  I.e., where is it defined and why does it affect only the 
breakpad component?


http://gerrit.cloudera.org:8080/#/c/5620/3/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

Line 244: 
nit: an extra-line, seems like a random change


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] log block manager: update list of buggy el6 versions

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: log_block_manager: update list of buggy el6 versions
..


Patch Set 1: Code-Review+1

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

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

[kudu-CR] Kudu Jepsen Tests - Initial Commit

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Kudu Jepsen Tests - Initial Commit
..


Patch Set 12:

Hrm, it sounds like the subsequent patch rewrites/improves a lot of what's in 
this one. Why not squash the patches so that reviewers don't have to spend time 
looking at stuff that's already been removed/changed?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] log block manager: update list of buggy el6 versions

[Todd Lipcon (Code Review)]

Hello Dan Burkert, Adar Dembo,

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

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

to review the following change.

Change subject: log_block_manager: update list of buggy el6 versions
..

log_block_manager: update list of buggy el6 versions

The bug fix for ext4 extents tree corruption has been backported to the
el6.8 update stream (aka z-stream). This adds the appropriate logic to
check for that version.

Change-Id: Id22c1d2527216620b6fc98b7a36fdbd552b72ddc
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
2 files changed, 32 insertions(+), 8 deletions(-)


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

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

[kudu-CR] [spark] KUDU-1631 push down StringStartsWith filters

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged.

Change subject: [spark] KUDU-1631 push down StringStartsWith filters
..


[spark] KUDU-1631 push down StringStartsWith filters

This adds support for pushing down StringStartsWith filters to
Kudu. It converts the prefix query into a range query.

Change-Id: Ieb50486fe3f3b44a241a554a953e4e2c81f17be4
Reviewed-on: http://gerrit.cloudera.org:8080/5461
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
M 
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 70 insertions(+), 5 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ieb50486fe3f3b44a241a554a953e4e2c81f17be4
Gerrit-PatchSet: 4
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 

[kudu-CR] [spark] KUDU-1631 push down StringStartsWith filters

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [spark] KUDU-1631 push down StringStartsWith filters
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb50486fe3f3b44a241a554a953e4e2c81f17be4
Gerrit-PatchSet: 3
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: No

[kudu-CR] KuduRDD.collect fails because of NoSerializableException

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KuduRDD.collect fails because of NoSerializableException
..


Patch Set 4:

https://issues.apache.org/jira/browse/KUDU-1824

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0463424481a33c66fd7464345c305062420cfe9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Kousuke Saruta 
Gerrit-Reviewer: Chris George 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kousuke Saruta 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KuduRDD.collect fails because of NoSerializableException

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: KuduRDD.collect fails because of NoSerializableException
..


Patch Set 4:

I've filed an issue and added some notes in 
https://issues.apache.org/jira/browse/KUDU-1824.  Now I'm going to try and 
figure out why the DataFrame API is not affected, and evaluate Kousuke's 
suggestion.  Will update the JIRA with findings.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0463424481a33c66fd7464345c305062420cfe9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Kousuke Saruta 
Gerrit-Reviewer: Chris George 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kousuke Saruta 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Kudu Jepsen Tests - Initial Commit

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Kudu Jepsen Tests - Initial Commit
..


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5492/12/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

Line 90: echo "Outputting java 8 home"
I agree with Adar's comment on rev 9 about removing or consolidating this to 1 
line.


PS12, Line 358: /opt/apache-maven-3.3.9/bin
> Are we ready to migrate to the new maven to build everything Java?
I tend to agree - just because we use JDK 8 to compile kudu-client and the rest 
doesn't mean they lose compatibility with JRE 7.  The -target flag to javac & 
maven can ensure compat.

Even moreso with bumping maven - as far as I know there aren't any hazards to 
packaging with maven 3.3 which would make the jar incompatible with maven 3.2 
or below.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add Google Breakpad support to Kudu

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change.

Change subject: Add Google Breakpad support to Kudu
..


Patch Set 3:

(3 comments)

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

Line 66: 
You might want to add a test for writing minidumps on SIGUSR1, especially since 
that would kill the process if the signal handler hasn't been set up correctly.


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

PS3, Line 56: nly be written when a daemon exits "
: "unexpectedly,
This is actually not true anymore (but used to be), even for Impala, since 
sending SIGUSR1 will trigger a Minidump (I opened IMPALA-4736).


Line 207:   if (!FLAGS_enable_minidumps || FLAGS_minidump_path.empty()) return 
Status::OK();
If you skip registering the signal handler for SIGUSR1 here and someone sends 
that signal, then the process will die. I noticed this while reviewing your 
code and opened IMPALA-4737 to track it on our end.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes