Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #10264 (closed enhancement: fixed)

Opened 8 months ago

Last modified 5 months ago

[PATCH] Throttle capistrano to only run against max_hosts connections at once

Reported by: robholland Assigned to: minam
Priority: normal Milestone:
Component: Capistrano Version:
Severity: normal Keywords:
Cc: jamis@37signals.com

Description

Attached is a patch which adds the max_hosts option to tasks which will limit capistrano to only running tasks against machines with max_hosts concurrent ssh connections at once. After each max_hosts-sized-block is done, it will tear down the ssh connections, and open the next max_hosts-sized-block.

Not sure if this is the best way to go about doing it, but we have a use case where we need to operate on a rather large set of machines and we were bumping up against outbound connection limits.

Thoughts?

I also removed a line from the existing tests related to the previous patches I sent in capistrano for on_error => continue. There was a .receive(:failed) which was causing test failures with my new patch. I cannot see how it ever worked, but it did. I'm a bit lost as to why that line was there and how the test passed with it. It looks wrong to me... weirdness.

Acknowledgement for: Rob Holland <rob@inversepath.com> if committed please.

Attachments

cap.max_hosts.patch (3.9 kB) - added by robholland on 11/24/07 09:00:06.
Patch
cap.patch (6.3 kB) - added by robholland on 02/21/08 10:39:27.

Change History

11/24/07 09:00:06 changed by robholland

  • attachment cap.max_hosts.patch added.

Patch

01/30/08 06:31:46 changed by minam

I'm still not entirely certain how I feel about closing the sessions...but maybe there's no other way when dealing with large numbers of connections. Is the concern here that you are hitting a restriction on maximum number of open connections, or is it something else?

01/30/08 06:32:06 changed by minam

  • cc set to jamis@37signals.com.

01/30/08 08:54:56 changed by robholland

Yes, the issue is the number of open sessions, coupled with wanting some control over the load required to process the in-flight replies.

01/30/08 08:55:22 changed by robholland

Open connections even.

01/30/08 10:03:58 changed by robholland

I should also note, that even better than the solution I proposed would be a thread-pool style solution instead of running in slices that would give better performance, rather than always waiting on the slowest of a slice before starting the next slice. I couldn't see how to implement that though.

02/21/08 04:57:09 changed by minam

  • status changed from new to assigned.

I'm still really debating what to do about this patch. I can see how this would be useful if you were dealing with very large numbers of servers, and I'm excited to see people using Capistrano to manage that, but somehow this feels messy. I looked at your patch long and hard tonight, trying to pinpoint why it makes me uneasy, and I think it just comes down to the fact that the code is overwhelming me. :) Could you try refactoring things into methods so that the contents of the each_slice call is a bit more manageable?

I'm really pleased that you've got tests and everything in this patch--so few people even try to do that. If you can clean up the implementation a little bit, I'll apply it, because I don't have any better ideas for how to do what you want to do. :)

02/21/08 10:39:27 changed by robholland

  • attachment cap.patch added.

02/21/08 10:40:46 changed by robholland

I've refactored a little, I hope this is what you had in mind. I agree the solution isn't ideal, but I couldn't find a nicer way.

02/22/08 04:07:20 changed by minam

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [8924]) Add support for :max_hosts option in task definition or run() (closes #10264)