You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/01/10 15:53:56 UTC
[kudu-CR] [scripts] Add initial test scripts for backup/restore testing
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12212
Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................
[scripts] Add initial test scripts for backup/restore testing
Adds a few rough scripts I am using to test the backup
and restore functionality. These scripts are rough
around the edges but I would like to get a base
script committed for collaboration purposes.
Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
---
A src/kudu/scripts/backup-perf.py
A src/kudu/scripts/grep_stats.py
A src/kudu/scripts/run_backup_tests.sh
3 files changed, 431 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/12212/1
--
To view, visit http://gerrit.cloudera.org:8080/12212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [scripts] Add initial test scripts for backup/restore testing
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12212 )
Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................
Patch Set 1:
(8 comments)
I rebased and addressed the quick and easy reviews. I dropped the less reusable scripts and opted to keep just the backup-perf.py script to get this in. This script has been used by at least 3 people for backup/restore testing already so it makes sense to commit.
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py
File src/kudu/scripts/backup-perf.py:
PS1:
> Some of the lines in this file are too long.
Done
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@223
PS1, Line 223: kudu-backup2_2.11-*.jar
> Could we glob this even more aggressively so that we're resilient to Spark/
Done
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/grep_stats.py
File src/kudu/scripts/grep_stats.py:
PS1:
> License header.
Done
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/grep_stats.py@6
PS1, Line 6: for line in sys.stdin:
> This script isn't used yet, and I think you could replace it with an awk sn
Done
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh
File src/kudu/scripts/run_backup_tests.sh:
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh@51
PS1, Line 51: NUM_TABLET_SERVERS=$((`kudu tserver list localhost -format=csv | wc -l`))
> This should be $KUDU_MASTER_ADDRESSES
Done
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh@62
PS1, Line 62: OPTS="$OPTS --kudu-spark-tools-jar $ROOT/java/kudu-spark-tools/build/libs/kudu-spark2-tools_2.11-1.9.0-SNAPSHOT.jar"
: OPTS="$OPTS --kudu-backup-jar $ROOT/java/kudu-backup/build/libs/kudu-backup2_2.11-1.9.0-SNAPSHOT.jar"
> Could we glob these so we needn't update this when the version changes?
Done
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh@81
PS1, Line 81: hadoop fs -rm -r -skipTrash $BACKUP_PATH/$TS
> Should we do something like this before we get started, to clear out any le
Done
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh@89
PS1, Line 89: exit 0
> This isn't necessary; the script will exit with status 0 on its own at the
Done
--
To view, visit http://gerrit.cloudera.org:8080/12212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 22 Mar 2019 15:10:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] [scripts] Add initial test scripts for backup/restore testing
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12212 )
Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................
[scripts] Add initial test scripts for backup/restore testing
Adds a rough script to test the backup and restore
functionality. This script is rough around the edges but
I would like to get a base script committed for collaboration purposes.
Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Reviewed-on: http://gerrit.cloudera.org:8080/12212
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
A src/kudu/scripts/backup-perf.py
1 file changed, 343 insertions(+), 0 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/12212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [scripts] Add initial test scripts for backup/restore testing
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12212 )
Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................
Patch Set 1: Code-Review+2
:shipit:
--
To view, visit http://gerrit.cloudera.org:8080/12212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 11 Jan 2019 02:00:57 +0000
Gerrit-HasComments: No
[kudu-CR] [scripts] Add initial test scripts for backup/restore testing
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12212 )
Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................
Patch Set 1:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py
File src/kudu/scripts/backup-perf.py:
PS1:
Some of the lines in this file are too long.
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@50
PS1, Line 50: self.last_tick_ = timeit.default_timer()
Nit: I don't think we need the underscore suffix to denote class variables; in Python you have to prepend 'self.' to every class variable reference anyway, so it's usually pretty obvious when you're accessing a class variable.
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@71
PS1, Line 71: Backported from Python 2.7 as it's implemented as pure python on stdlib.
Not really understanding the justification. I thought this was so you could use check_output() on Python 2.6, since it was introduced in 2.7?
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@76
PS1, Line 76: output, unused_err = process.communicate()
Nit: Python convention is to use _ in place of a variable whose value you don't care about.
But if this is a faithful backport, ignore this comment.
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@223
PS1, Line 223: kudu-backup2_2.11-*.jar
Could we glob this even more aggressively so that we're resilient to Spark/Scala version changes too?
For kudu-spark2-tools as well.
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@244
PS1, Line 244: # Actions
I think you can simplify the true/false stuff with action='store_true'; doesn't that imply type, choices, and default?
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/grep_stats.py
File src/kudu/scripts/grep_stats.py:
PS1:
License header.
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/grep_stats.py@6
PS1, Line 6: for line in sys.stdin:
This script isn't used yet, and I think you could replace it with an awk snippet anyway. Something like https://stackoverflow.com/questions/38972736/how-to-select-lines-between-two-patterns/38972737#38972737.
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh
File src/kudu/scripts/run_backup_tests.sh:
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh@51
PS1, Line 51: NUM_TABLET_SERVERS=$((`kudu tserver list localhost -format=csv | wc -l`))
This should be $KUDU_MASTER_ADDRESSES
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh@62
PS1, Line 62: OPTS="$OPTS --kudu-spark-tools-jar $ROOT/java/kudu-spark-tools/build/libs/kudu-spark2-tools_2.11-1.9.0-SNAPSHOT.jar"
: OPTS="$OPTS --kudu-backup-jar $ROOT/java/kudu-backup/build/libs/kudu-backup2_2.11-1.9.0-SNAPSHOT.jar"
Could we glob these so we needn't update this when the version changes?
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh@81
PS1, Line 81: hadoop fs -rm -r -skipTrash $BACKUP_PATH/$TS
Should we do something like this before we get started, to clear out any leftover state from the a previous interrupted run?
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/run_backup_tests.sh@89
PS1, Line 89: exit 0
This isn't necessary; the script will exit with status 0 on its own at the end.
--
To view, visit http://gerrit.cloudera.org:8080/12212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 11 Jan 2019 04:25:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] [scripts] Add initial test scripts for backup/restore testing
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12212
to look at the new patch set (#2).
Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................
[scripts] Add initial test scripts for backup/restore testing
Adds a rough script to test the backup and restore
functionality. This script is rough around the edges but
I would like to get a base script committed for collaboration purposes.
Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
---
A src/kudu/scripts/backup-perf.py
1 file changed, 343 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/12212/2
--
To view, visit http://gerrit.cloudera.org:8080/12212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] [scripts] Add initial test scripts for backup/restore testing
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12212 )
Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 22 Mar 2019 17:42:46 +0000
Gerrit-HasComments: No
[kudu-CR] [scripts] Add initial test scripts for backup/restore testing
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12212 )
Change subject: [scripts] Add initial test scripts for backup/restore testing
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py
File src/kudu/scripts/backup-perf.py:
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@71
PS1, Line 71: Backported from Python 2.7 as it's implemented as pure python on stdlib.
> Not really understanding the justification. I thought this was so you could
Yeah this allows us to run this script on RHEL6.6
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@76
PS1, Line 76: output, unused_err = process.communicate()
> Nit: Python convention is to use _ in place of a variable whose value you d
This is a straight backport / copy from build-support, copied so this script could be copied over to a host and used
http://gerrit.cloudera.org:8080/#/c/12212/1/src/kudu/scripts/backup-perf.py@244
PS1, Line 244: # Actions
> I think you can simplify the true/false stuff with action='store_true'; doe
I tried that and found it confusing but maybe Grant knows this stuff better than me
--
To view, visit http://gerrit.cloudera.org:8080/12212
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c8efc3778b20687f0c1bc4c825f49f8f24e6d3b
Gerrit-Change-Number: 12212
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 11 Jan 2019 21:29:05 +0000
Gerrit-HasComments: Yes