Add a feature: connect to Unix Domain Sockets

[pzillmann]

Connecting to Unix Domain Sockets e.g. /var/run/mongodb/mongod.sock

Example connection string user:pass@/var/run/mongodb/mongod.sock/database

[domodwyer]

Hi @pzillmann

This is a really great addition and it's kinda weird it was never there in the first place - thanks! We'll take a look and get it merged :)

In the meantime would you mind adding a reference to this in the dial string documentation as part of this PR?

Thanks for including tests!

Dom

[pzillmann]

@domodwyer in the year 2012 somebody tried to add that feature into the labix' mgo driver - seems to be forgotten.

https://bugs.launchpad.net/mgo/+bug/954436 https://jira.mongodb.org/browse/TOOLS-620

@szank thank you for your feedback. I definitely have to learn more about interfaces. I changed the typecast in the address resolver back to tcp, so dial.old doesn't have to do that. In the end I got cleaner code with less changes.

[domodwyer]

Hi @pzillmann

First off, thanks for the hard work - it's truly appreciated.

Unfortunately though you've fallen into some of the oldest, most questionable code in the mgo driver - it's a project that's been going on for a long time (since at least 2010, so pre Go 1 I think) so there's lots of stuff in there that has been solved much more elegantly by the Go stdlib, or accepted conventions have come about that mgo misses - it makes contributions more difficult than they should be, so I'm really sorry about that.

The feature is definitely wanted - I left some PR comments that should make it possible to take an "easier" path through the old code, and I included some examples/references to docs to help you along, I hope they help!

Dom

[pzillmann]

@domodwyer Thanks for the feedback. So you want to get rid of some old code? There might be some giant pitfalls with some tests. I would like to have your opinion / some direction.

• replace the "UDP dialing hack"?!
• IPv6 > IPv4 (or at least don't resolve the address and let decide the system's DNS resolver what address family to use) [the timeout + IPv6 code got implemented in 2015...]?!
• decide Unix Socket or IP with a string analyses?!

When you are talking about obsolete things, are there any obsolete tests? I may see what I can do.

[domodwyer]

Hi @pzillmann

I would suggest getting this feature added first and keep the scope of the PR small - I would do something like:

func resolveAddr(addr string) (net.Addr, error) {
    if isUnixSocketAddr(addr) {
        return &net.UnixAddr{Net: "unix", Name: addr}
    }

    // ...rest of code...
}

This should let you keep the changes to resolveAddr() to the minimum and we can get this PR merged (with the URL encode thing too) - it would be a great feature to add to the driver!

If you fancy modernising some of the code in a different PR that would be awesome - things like removing mongoServer.ResolvedAddr completely (I can't see why it's there?), using IPv6 without a 50ms delay, using channels with a close() to signal the end of resolution, sync.WaitGroup where appropriate, a context.Context to signal to the other resolver goroutines to stop, etc. If this is something you're interested in please do open a "review connection handling and DNS resolution" issue and we can discuss as a group what would be best 👍

Either way, thanks again! Dom

[pzillmann]

I've taken the suggested changes into account. Hopefully I did not miss something. Resolved merge conflicts + every host is checked for unescaped characters.

Edit: Test Job 484.5 failed with

--- FAIL: TestCoarseTimeProvider (1.00s)
    coarse_time_test.go:21: got 1529180370, expected at least 1529180370

What was wrong there - expected value is equal to given value?

[domodwyer]

This looks great!

Thanks @pzillmann, we really appreciate the time you've put into this 👍

Dom

[domodwyer]

Thanks very much @pzillmann!