celluloid / celluloid-io

UNMAINTAINED: See celluloid/celluloid#779 - Evented sockets for Celluloid actors
https://celluloid.io
MIT License
879 stars 93 forks source link

Net::SSH not fully compatible with Non-blocking IO; How to workaround? #125

Closed HoneyryderChuck closed 9 years ago

HoneyryderChuck commented 9 years ago

See:

(net-ssh 2.7, even though master is no different) https://github.com/net-ssh/net-ssh/blob/2.7/lib/net/ssh/ruby_compat.rb#L34-L46 https://github.com/net-ssh/net-ssh/blob/2.7/lib/net/ssh/connection/session.rb#L209

This method is then used in net-ssh-telnet like this: https://github.com/jasonkarns/net-ssh-telnet/blob/master/lib/net/ssh/telnet.rb#L357

Which means, the Net::SSH::Telnet client with an open Net::SSH session below patched with the Celluloid::IO TCPSocket will still call IO.select, which blocks.

How can I "simulate" the IO::select call in the net ssh session using Celluloid::IO? I obviously have to patch this locally so that this is fully Celluloid::IO compliant, but this also may mean that Net::SSH is non-blocking-non-compliant, or am I missing something here?

I'll open the ticket in net-ssh after I get the confirmation that there is nothing celluloid-io can do besides injecting the socket.

ioquatix commented 9 years ago

I think you'd need to fix net-ssh. You might be able to monkey patch the Net::SSH namespace to replace IO.

ioquatix commented 9 years ago

Just as an aside: perhaps it's worth making Celluloid::SSH if it's a really useful use-case.

HoneyryderChuck commented 9 years ago

@ioquatix , I've been indeed monkey-patching net-ssh to fit (my) celluloid's needs. We're rolling out the changes to production soon, so I'll have an opportunity to see it work. And yes, I've been thinking about a celluloid-ssh project (and a celluloid-telnet one) as soon as I'm confident enough. As for now, this is just a reminder take net-ssh out from here or at least to set an alert.

fascox commented 9 years ago

@TiagoCardoso1983 how did you patched IO.select ? it worked?

tarcieri commented 9 years ago

You might want to check out this nascent project:

https://github.com/zanker/socketry/tree/master/lib/socketry

It's a set of wrappers that handle things like timeouts in a generic way across core Ruby or Celluloid::IO

HoneyryderChuck commented 9 years ago

@tarcieri is this something akin to the timeout-extensions gem?

tarcieri commented 9 years ago

@TiagoCardoso1983 more like trying to avoid using Timeout at all, instead using asynchronous I/O-driven timeouts both with native IO objects and with Celluloid::IO

HoneyryderChuck commented 9 years ago

@fascox I didn't patch IO.select, I monkey-patched the parts of net-ssh which are using it. I think net-ssh needs a bit of a rewrite if it wants to integrate better with network concurrency frameworks like celluloid-io or even eventmachine.

Asmod4n commented 9 years ago

It's embarrassing how easy it is to handle timeouts in C..

fascox commented 9 years ago

@TiagoCardoso1983 thank you. if you could share some bits of the patch or point me to right direction to make it non-blocking is appreciated. I'm using net::ssh inside an actor to fire remote shell commands to multiple server. My requirements is that actors (ssh command) could be terminated on demand, but blocking IO freeze mailboxes and prevent me to terminate before it completes. A workaround was to isolate the call to net::ssh inside another actor, out of the logic of the worker (sidekiq), but i'm looking for a better and clean solution.

HoneyryderChuck commented 9 years ago

@fascox I'll see if I can do that as soon as I return from the holidays. But yes, your use case is exactly the one I already solved. Currently I'm using N=number-of-cores actors, and I distribute the hosts among them and let them work. I support both telnet and ssh. It's performing very well, just a bit below the previous solution using eventmachine. But I get the benefit of more maintainable code (eventmachine with ssh also requires patches).