You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2016/09/15 06:48:21 UTC

[kudu-CR] WIP: Implement an upgrade test

Mike Percy has uploaded a new change for review.

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

Change subject: WIP: Implement an upgrade test
......................................................................

WIP: Implement an upgrade test

This program takes two binary directories and runs a simple "upgrade"
test between the two versions to ensure that a table created with
"Version A" of the software can be read by "Version B" of the software.

TODO: Change the cmake config so that it doesn't get run as a unit test
but still can use gtest / KuduTest.

As a follow-up, we would benefit from a higher-level script that can
manage upgrade and downgrade testing across many versions of Kudu.

Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/upgrade-test.cc
4 files changed, 166 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>

[kudu-CR] Implement an upgrade test

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.

Change subject: Implement an upgrade test
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4424/2/src/kudu/integration-tests/upgrade-test.cc
File src/kudu/integration-tests/upgrade-test.cc:

PS2, Line 108: FLAGS_test_version_a_bin
Typo: FLAGS_test_version_b_bin


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] Implement a version migration test

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: Implement a version migration test
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4424/3/src/kudu/integration-tests/version_migration-test.cc
File src/kudu/integration-tests/version_migration-test.cc:

Line 115:   NO_FATALS(cluster_->AssertNoCrashes());
Consider adding a check that the new cluster can start next time successfully as well.  I don't know the internals, but imagine the new version loads old data and then stores the updated data with new layout/representation.  In that situation, I think it's worth checking that the updated data has been written successfully and the next time processes start up, they can read what have been written.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] Implement an upgrade test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: Implement an upgrade test
......................................................................


Patch Set 2:

(12 comments)

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

PS2, Line 7: upgrade
> Wouldn't this also be a downgrade test if we happen to use the binary-set B
Done


PS2, Line 9: a simple
> This is bit more white-boxy than I initially thought. Thinking out loud her
I don't think there are currently any integration tests that could be repurposed in the way you are describing. If that is true, then we have to write new tests.

This first test is very "white box" but that doesn't mean we can't easily write more "black box" tests using the existing tools / libraries we already use for integration testing in the rest of the code base.


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

Line 255:   void SetDaemonBinPath(std::string daemon_bin_path);
> Nit: name it to SetBinaryPath in comparison to GetBinaryPath below?
I don't think that would make sense. GetBinaryPath() takes the name of a binary to resolve into an absolute path based on what the daemon_bin_path was set to.


http://gerrit.cloudera.org:8080/#/c/4424/2/src/kudu/integration-tests/upgrade-test.cc
File src/kudu/integration-tests/upgrade-test.cc:

PS2, Line 18: #include <gflags/gflags.h>
            : #include <gtest/gtest.h>
> nit: consider listing these after STL includes, as recommended by the style
Done


Line 22: #include <unordered_map>
> nit: is this really needed for the code below?
Done


Line 38: DEFINE_int32(num_snapshots, 3, "Number of snapshots to verify across replicas and reboots.");
> nit: s/num_snapshots/num_samples ?
I just copy / pasted this from linked_list-test so I think it's reasonable to keep the flags named the same.


Line 41: using std::unique_ptr;
> consider adding
Done


PS2, Line 53:     if (FLAGS_test_version_a_bin.empty() || FLAGS_test_version_b_bin.empty()) {
            :       LOG(FATAL) << "Must specify --test_version_a_bin and --test_version_b_bin flags";
            :     }
> nit: does it make  sense to check for this first before running KuduTest::S
Done


PS2, Line 58: std::vector<std::string>()
> nit: you could use c++11 features to shorten the signature, i.e. use '{}' i
cool, never done that for default args


PS2, Line 76: undefok
> nit: do we need undefok here and below ?
Yes, we need it because those flags are not in older versions of kudu-tserver and kudu-master, but the ExternalMiniCluster passes them into the binary when invoking them.


Line 104: 
> Do you think it's worth verifying that the server with the same version is 
Yes, that's already being done with the call to WaitAndVerify()


PS2, Line 108: FLAGS_test_version_a_bin
> Typo: FLAGS_test_version_b_bin
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] Implement a version migration test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: Implement a version migration test
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4424/3/src/kudu/integration-tests/version_migration-test.cc
File src/kudu/integration-tests/version_migration-test.cc:

Line 115:   NO_FATALS(cluster_->AssertNoCrashes());
> Consider adding a check that the new cluster can start next time successful
Maybe something like that should be written as an additional targeted test that tries to exercise specific code paths that we think are potentially problematic?

The only data guaranteed to be new after a restart is the superblock (exercised by WaitAndVerify() since you can't find any data without going through the superblock) and the WAL on each tablet. I suppose I could add a call to LogVerifier though. I'll do that.

IMHO this is already the most basic useful version migration test. We could and should add lots more "upgrade" tests to this test suite, but I don't think we should try to catch every possible issue in this single test.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] Implement a version migration test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Dinesh Bhat,

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

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

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

Change subject: Implement a version migration test
......................................................................

Implement a version migration test

This program takes two binary directories and runs a simple "migration"
test between the two versions to ensure that a table created with
"Version A" of the software can be read by "Version B" of the software.
This can also be thought of as an "upgrade" or "downgrade" test.

This patch also makes some changes to ExternalMiniCluster and the cmake
files to allow for creating an executable that is written like a unit
test (using the Google Test framework) but is not automatically executed
by ctest.

As a follow-up, we would benefit from a higher-level script that can
manage upgrade and downgrade testing across many versions of Kudu.

Tested with Version A = 0.10.0 and Version B = 1.0.0

Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
---
M CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/version_migration-test.cc
5 files changed, 168 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] Implement an upgrade test

Posted by "Dinesh Bhat (Code Review)" <ge...@cloudera.org>.
Dinesh Bhat has posted comments on this change.

Change subject: Implement an upgrade test
......................................................................


Patch Set 2:

(5 comments)

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

PS2, Line 7: upgrade
Wouldn't this also be a downgrade test if we happen to use the binary-set B from lower release number ?


PS2, Line 9: a simple
This is bit more white-boxy than I initially thought. Thinking out loud here, I am not sure if it's possible with gtest to do this or if this is complicated: Keep this UpgradeTest as a wrapper(or a co-ordinator) between 2 integration-test runs, i.e given an integration-test name provided as argument, upgrade test runs the specified test using both release bits. Given that each of these tests run entirely independently and setup the clusters from clean slate, this seems not so plausible at the moment. Perhaps this could have been possible if we had stashed away ExternalMiniCluster config in a persistent location(purely as a means to communicate to next run of the same test). Whole point of this idea was to try to cover compatibility for several other subsystems like FS, PB, etc without needing to add newer set of tests just for the upgrade test use case. Instead leverage the existing suite of tests and invoke them using upgrade test.


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

Line 255:   void SetDaemonBinPath(std::string daemon_bin_path);
Nit: name it to SetBinaryPath in comparison to GetBinaryPath below?


http://gerrit.cloudera.org:8080/#/c/4424/2/src/kudu/integration-tests/upgrade-test.cc
File src/kudu/integration-tests/upgrade-test.cc:

Line 38: DEFINE_int32(num_snapshots, 3, "Number of snapshots to verify across replicas and reboots.");
nit: s/num_snapshots/num_samples ?


PS2, Line 76: undefok
nit: do we need undefok here and below ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] Implement an upgrade test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Implement an upgrade test
......................................................................

Implement an upgrade test

This program takes two binary directories and runs a simple "upgrade"
test between the two versions to ensure that a table created with
"Version A" of the software can be read by "Version B" of the software.

This patch also makes some changes to ExternalMiniCluster and the cmake
files to allow for creating an executable that is written like a unit
test (using the Google Test framework) but is not automatically executed
by ctest.

As a follow-up, we would benefit from a higher-level script that can
manage upgrade and downgrade testing across many versions of Kudu.

Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
---
M CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/upgrade-test.cc
5 files changed, 158 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] Implement a version migration test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: Implement a version migration test
......................................................................


Patch Set 3: Verified+1

Overriding flaky java test

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] Implement an upgrade test

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: Implement an upgrade test
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4424/2/src/kudu/integration-tests/upgrade-test.cc
File src/kudu/integration-tests/upgrade-test.cc:

PS2, Line 18: #include <gflags/gflags.h>
            : #include <gtest/gtest.h>
nit: consider listing these after STL includes, as recommended by the style guide .


Line 22: #include <unordered_map>
nit: is this really needed for the code below?


Line 41: using std::unique_ptr;
consider adding

using std::vector and
using std::string

as well and then dropping the std:: prefix in all places using those classes.


PS2, Line 53:     if (FLAGS_test_version_a_bin.empty() || FLAGS_test_version_b_bin.empty()) {
            :       LOG(FATAL) << "Must specify --test_version_a_bin and --test_version_b_bin flags";
            :     }
nit: does it make  sense to check for this first before running KuduTest::SetUp()?


PS2, Line 58: std::vector<std::string>()
nit: you could use c++11 features to shorten the signature, i.e. use '{}' instead of 'std::vector<std::string>()' for the default parameters.


Line 104: 
Do you think it's worth verifying that the server with the same version is able to read what's has been written, prior to an attempt to read the data with the upgraded version?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] Implement a version migration test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Implement a version migration test
......................................................................

Implement a version migration test

This program takes two binary directories and runs a simple "migration"
test between the two versions to ensure that a table created with
"Version A" of the software can be read by "Version B" of the software.
This can also be thought of as an "upgrade" or "downgrade" test.

This patch also makes some changes to ExternalMiniCluster and the cmake
files to allow for creating an executable that is written like a unit
test (using the Google Test framework) but is not automatically executed
by ctest.

As a follow-up, we would benefit from a higher-level script that can
manage upgrade and downgrade testing across many versions of Kudu.

Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
---
M CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/version_migration-test.cc
5 files changed, 161 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] Implement a version migration test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: Implement a version migration test
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4424/3/src/kudu/integration-tests/version_migration-test.cc
File src/kudu/integration-tests/version_migration-test.cc:

Line 115: 
> Maybe something like that should be written as an additional targeted test 
OK, on second thought, even though we have pretty good coverage now maybe someday we will change things. So being paranoid doesn't hurt very much. Added a loop to the restart / verification process here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1596. Implement a version migration test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Dinesh Bhat, Kudu Jenkins,

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

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

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

Change subject: KUDU-1596. Implement a version migration test
......................................................................

KUDU-1596. Implement a version migration test

This program takes two binary directories and runs a simple "migration"
test between the two versions to ensure that a table created with
"Version A" of the software can be read by "Version B" of the software.
This can also be thought of as an "upgrade" or "downgrade" test.

This patch also makes some changes to ExternalMiniCluster and the cmake
files to allow for creating an executable that is written like a unit
test (using the Google Test framework) but is not automatically executed
by ctest.

As a follow-up, we would benefit from a higher-level script that can
manage upgrade and downgrade testing across many versions of Kudu.

Tested with Version A = 0.10.0 and Version B = 1.0.0

Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
---
M CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/version_migration-test.cc
5 files changed, 174 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-1596. Implement a version migration test

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1596. Implement a version migration test
......................................................................


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] Implement a version migration test

Posted by "Dinesh Bhat (Code Review)" <ge...@cloudera.org>.
Dinesh Bhat has posted comments on this change.

Change subject: Implement a version migration test
......................................................................


Patch Set 3: Code-Review+1

(4 comments)

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

PS2, Line 9: a simple
> I don't think there are currently any integration tests that could be repur
Yeah agree, no doubt this is a very good first step in that direction.


http://gerrit.cloudera.org:8080/#/c/4424/3//COMMIT_MSG
Commit Message:

PS3, Line 7: Implement
Would you like to specify the JIRA number as well here ?


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

Line 255:   void SetDaemonBinPath(std::string daemon_bin_path);
> I don't think that would make sense. GetBinaryPath() takes the name of a bi
Sounds good, thanks for clarifying.


http://gerrit.cloudera.org:8080/#/c/4424/2/src/kudu/integration-tests/upgrade-test.cc
File src/kudu/integration-tests/upgrade-test.cc:

Line 38
> I just copy / pasted this from linked_list-test so I think it's reasonable 
True, I think it could be reworded in the linked_list-test too, because the actual definition of LoadLinkedTest calls it as 'num_samples'. Also the term 'snapshot' is sometimes context specific and tends to get overloaded in the code, say for eg if we had a snapshot feature for a kudu object. However, I don't feel strongly about this since this flag is test-specific anyways.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1596. Implement a version migration test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged.

Change subject: KUDU-1596. Implement a version migration test
......................................................................


KUDU-1596. Implement a version migration test

This program takes two binary directories and runs a simple "migration"
test between the two versions to ensure that a table created with
"Version A" of the software can be read by "Version B" of the software.
This can also be thought of as an "upgrade" or "downgrade" test.

This patch also makes some changes to ExternalMiniCluster and the cmake
files to allow for creating an executable that is written like a unit
test (using the Google Test framework) but is not automatically executed
by ctest.

As a follow-up, we would benefit from a higher-level script that can
manage upgrade and downgrade testing across many versions of Kudu.

Tested with Version A = 0.10.0 and Version B = 1.0.0

Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Reviewed-on: http://gerrit.cloudera.org:8080/4424
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
A src/kudu/integration-tests/version_migration-test.cc
5 files changed, 174 insertions(+), 5 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] Implement a version migration test

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: Implement a version migration test
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4424/3//COMMIT_MSG
Commit Message:

PS3, Line 7: Implement
> Would you like to specify the JIRA number as well here ?
Done


http://gerrit.cloudera.org:8080/#/c/4424/2/src/kudu/integration-tests/upgrade-test.cc
File src/kudu/integration-tests/upgrade-test.cc:

Line 38: DEFINE_int32(num_snapshots, 3, "Number of snapshots to verify across replicas and reboots.");
> True, I think it could be reworded in the linked_list-test too, because the
If you don't mind I'd like to punt on this, so I don't have to hunt down Jenkins jobs to change the flags in.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2831b47e6c0b644a256e1914fa495f453318e0cd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes