Browse files

Make confirm! and on_all_servers pruning more testable, and test them.

  • Loading branch information...
1 parent 2195d7a commit f6b206eb0261c220e6c7b98a95e0cf438fcfed73 @stuhood stuhood committed Apr 11, 2012
Showing with 143 additions and 43 deletions.
  1. +1 −1 Rakefile
  2. +15 −9 lib/gizzard/commands.rb
  3. +75 −33 lib/gizzard/nameserver.rb
  4. +13 −0 test/gizzmo_spec.rb
  5. +39 −0 test/nameserver_spec.rb
View
2 Rakefile
@@ -17,7 +17,7 @@ Jeweler::Tasks.new do |gem|
gem.name = "gizzmo"
gem.summary = %Q{Gizzmo is a command-line client for managing gizzard clusters.}
gem.description = %Q{Gizzmo is a command-line client for managing gizzard clusters.}
- gem.email = "kmaxwell@twitter.com"
+ gem.email = "stuhood@twitter.com"
gem.homepage = "http://github.com/twitter/gizzmo"
gem.authors = ["Kyle Maxwell"]
end
View
24 lib/gizzard/commands.rb
@@ -3,17 +3,23 @@
require "digest/md5"
module Gizzard
- def Gizzard::confirm!(force=false, message="Continue?")
+ CONFIRM_OPTIONS = Hash[
+ 'y' => lambda { true },
+ 'n' => lambda { puts "Exiting"; exit 1 }
+ ].freeze
+
+ # takes a map of 'character => lambda': if a character is defined, the result of
+ # executing the (no-arg) lambda is returned
+ def Gizzard::confirm!(force=false, message="Continue?", options=CONFIRM_OPTIONS, input=$stdin, output=$stdout)
return if force
+ opt_char_string = options.keys.join('/')
begin
- print "#{message} (y/n) "; $stdout.flush
- resp = $stdin.gets.chomp.downcase
- puts ""
- end while resp != 'y' && resp != 'n'
- if resp == 'n'
- puts "Exiting."
- exit
- end
+ output.print "#{message} (#{opt_char_string}) "
+ output.flush
+ resp = input.gets.chomp.downcase
+ output.puts ""
+ end while !options.has_key?(resp)
+ options[resp].call
end
class Command
View
108 lib/gizzard/nameserver.rb
@@ -89,6 +89,45 @@ class Nameserver
MAX_ATTEMPT_SECS = 30
PARALLELISM = 10
+ PRUNE_HOST_MSG =
+ "(r)etry on these hosts, (i)gnore these hosts for the remainder of the transform, (k)ill the process?"
+ PRUNE_HOST_OPTS = Hash[
+ 'r' => lambda { true },
+ 'i' => lambda { false },
+ 'k' => lambda { raise Exception.new("Killing transform.") }
+ ].freeze
+
+ # given a list of all_clients, and a list of triples of (client, result, exception),
+ # ask the user how to handle the failed clients, and return a tuple of
+ # (all_clients, failed_clients_to_consider_successful). If the user does not want
+ # to proceed or there are no hosts to continue with, raises an exception.
+ def Nameserver.prune_hosts(force, operation_name, all_clients, failed_client_triples, input=$stdin, output=$stdout)
+ output.puts "#{failed_client_triples.size} of #{all_clients.size} clients " +
+ "failed to execute '#{operation_name}':"
+ failed_client_triples.each do |client, _, exception|
+ output.puts "\t#{client.get_host} failed with: #{exception}"
+ end
+ if force
+ raise Exception.new("Cannot proceed past exceptions while force=true: exiting.")
+ end
+ res = Gizzard::confirm!(false, PRUNE_HOST_MSG, PRUNE_HOST_OPTS, input, output)
+
+ # we're still alive: user wanted to proceed, either by retrying failed hosts,
+ # or by pruning them
+ if res
+ # continue with full host list
+ [all_clients,[]]
+ else
+ # return an updated list
+ without_clients = failed_client_triples.map{|client, _, _| client }
+ res_all_clients = all_clients - without_clients
+ if res_all_clients.empty?
+ raise Exception.new("No viable clients remain: exiting.")
+ end
+ [res_all_clients, without_clients]
+ end
+ end
+
attr_reader :hosts, :logfile, :dryrun, :framed
alias dryrun? dryrun
@@ -192,43 +231,46 @@ def validate_clients_or_raise
# executes the given block in parallel with a client for each server: in the face of failure,
# may return less results than there are clients
def on_all_servers(operation_name, &block)
- # fork into many threads, and then join with exception handling
- clients_and_threads = all_clients.map do |client|
- [client, Thread.new { Thread.current[:result] = block.call(client) }]
- end
- clients_and_results_or_exceptions = clients_and_threads.map do |client, thread|
- begin
- thread.join
- [client, thread[:result], nil]
- rescue Exception => e
- [client, nil, e]
+ remaining_clients = all_clients
+ successful_results = []
+ while true do
+ # fork into many threads, and then join with exception handling
+ clients_and_threads = remaining_clients.map do |client|
+ [client, Thread.new { Thread.current[:result] = block.call(client) }]
end
- end
-
- successful_clients, failed_clients =
- clients_and_results_or_exceptions.partition{|_, _, exception| exception.nil? }
- if failed_clients.size > 0
- # if there were failed clients, but the user would like to proceed anyway,
- # mutate @all_clients to remove the failed clients
- puts "#{failed_clients.size} of #{all_clients.size} clients failed to execute '#{operation_name}':"
- failed_clients.each do |client, _, exception|
- puts "\t#{client.get_host} failed with: #{exception}"
+ clients_and_results_or_exceptions = clients_and_threads.map do |client, thread|
+ begin
+ thread.join
+ [client, thread[:result], nil]
+ rescue Exception => e
+ [client, nil, e]
+ end
end
- if @force
- puts "Cannot proceed past exceptions while force=true: exiting."
- exit 1
+
+ successful_clients, failed_clients =
+ clients_and_results_or_exceptions.partition{|_, _, exception| exception.nil? }
+ # collect successful results and remove successful clients
+ remaining_clients =
+ remaining_clients - successful_clients.map{|c, _, _| c }
+ successful_results =
+ successful_results + successful_clients.map{|_, r, _| r }
+
+ if failed_clients.size > 0
+ begin
+ # if there were failed clients, but the user would like to proceed anyway,
+ # mutate all_clients and remaining_clients
+ all_clients, considered_successful_clients =
+ Nameserver.prune_hosts(@force, operation_name, all_clients, failed_clients)
+ remaining_clients =
+ remaining_clients - considered_successful_clients.map{|c, _, _| c }
+ rescue Exception => e
+ puts "Did not complete '#{operation_name}': " + e
+ exit 1
+ end
end
- Gizzard::confirm!(false, "Proceed without these hosts?")
- # we're still alive: user wanted to proceed
- @all_clients.reject!(failed_clients.map{|client, _, _| client })
- end
- if @all_clients.size < 1
- puts "No viable clients remain: exiting."
- exit 1
- end
- # return only successful results
- successful_clients.map{|_, result, _| result }
+ return successful_results if remaining_clients.empty?
+ end
end
def client
View
13 test/gizzmo_spec.rb
@@ -16,6 +16,19 @@ def fuzzily(str)
@nameserver_db = nil
end
+ describe "confirm!" do
+ it "differentiates between inputs" do
+ opt = Hash['y' => lambda {|| 'y' }]
+ output = sio()
+ Gizzard::confirm!(false, "Blah?", opt, sio("n\ny\n"), output).should == 'y'
+ output.string.should == "Blah? (y) \nBlah? (y) \n"
+ end
+
+ def sio(string="")
+ StringIO.new(string)
+ end
+ end
+
describe "basic manipulation commands" do
describe "create" do
it "creates a single shard" do
View
39 test/nameserver_spec.rb
@@ -101,6 +101,45 @@ def parse_should(hostname, table_prefix)
it "works..."
end
+ describe "prune_hosts" do
+ class Client
+ def initialize(host)
+ @host = host
+ end
+
+ def get_host
+ @host
+ end
+ end
+
+ one = Client.new(1)
+ two = Client.new(2)
+ all = [one, two]
+ failed = [[one, "Result", "Exception"]]
+
+ it "ignores a failed client" do
+ prune("i\n", all, failed).should == [[two], [one]]
+ end
+
+ it "retries a failed client" do
+ prune("r\n", all, failed).should == [all, []]
+ end
+
+ it "fails when no more hosts" do
+ expect { prune("k\n", all, failed) }.should raise_error
+ end
+
+ it "fails when requested" do
+ expect { prune("k\n", all, failed) }.should raise_error
+ end
+
+ def prune(instr, all, failed)
+ input = StringIO.new(instr)
+ output = StringIO.new("")
+ Gizzard::Nameserver.prune_hosts(false, "prune", all, failed, input, output)
+ end
+ end
+
describe "reload_config" do
it "reloads config on every app server" do
mock(@client).reload_config

0 comments on commit f6b206e

Please sign in to comment.