Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unixsockets #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix unixsockets #111

wants to merge 2 commits into from

Conversation

guybrush
Copy link

  • reconnect now works properly with unixsockets
  • the callback function passed to socket.listen() will
    return the EADDRINUSE if the attempt to connect
    does not result in ECONNREFUSED
  • provide a callback to fs.link() when recycling the
    stale socket file (see 1)

} else {
// host = fn;
// fn = undefined;
port = port.pathname;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i forgot some // (will just push -f it away)

the whole option-parsing here is kind of weird but i feel like a small PR is better than refactoring all the things \o/

* reconnect now works properly with unixsockets
* the callback function passed to `socket.listen()` will
  return the `EADDRINUSE` if the attempt to connect
  does *not* result in `ECONNREFUSED`
* provide a callback to `fs.link()` when recycling the
  stale socket file (see [1])

[1]: https://github.com/joyent/node/blob/v0.10.21-release/lib/fs.js#L84
@tj
Copy link
Owner

tj commented Oct 30, 2013

haha ugh.. and the maintenance begins, I still sort of feel like unix domain sockets is a big meh, it's easy to do ephemeral stuff over tcp

@tj
Copy link
Owner

tj commented Oct 30, 2013

maybe we should revert that support

@guybrush
Copy link
Author

i looked into this lib because of unix-sockets in the first place, because i needed to do stuff on foreign system

i dont see why you wouldnt support them since its not so much weight, but its your call and im not the guy who stands behind the lib and ensures everything is ok :D

@tj
Copy link
Owner

tj commented Oct 30, 2013

i just feel like they're really minimal win, there's almost never a valid reason for using them instead of regular old tcp, not that they're bad either but it is more stuff that needs testing/maintenance/docs

@guybrush
Copy link
Author

hm maybe put it into another module which can be used to wrap axon?

var u = require('axon-unix')
var a = require('axon')
var s = a.socket('req')
var c = a.socket('rep')
u(s).bind('/path/to/socket')
u(c).connect('/path/to/socket')

@gjohnson
Copy link
Collaborator

Yeah, they are not horrible but I feel like the whole unlinking and what not will stir up all kinds of non-issues that will still need to be looked into.

@gjohnson
Copy link
Collaborator

gjohnson commented Feb 2, 2014

Since were trimming down for 2.0, we should back out unix sockets there probably. Thoughts?

@guybrush
Copy link
Author

guybrush commented Feb 2, 2014

better no unix-sockets than a half-implemented thing (imho)

@tj
Copy link
Owner

tj commented Feb 2, 2014

im down for removing them

@rlidwka
Copy link

rlidwka commented Feb 2, 2014

Err... don't remove that, pleeeease. I was about to suggest using unix sockets instead of tcp for pm2, and it'd kinda sad if it'll be removed. :P

In theory, with unix sockets it's easier to manage permissions, since socket is just a file. With tcp you need to set up an authentication and all that crap.

@sladec
Copy link

sladec commented Apr 12, 2016

Please don't remove unix sockets, better performance then tcp.
33% better ?http://stackoverflow.com/questions/257433/postgresql-unix-domain-sockets-vs-tcp-sockets/257479#257479
#150 seems to fix reconnect issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants