[nanomsg] Re: more updates
- From: Garrett D'Amore <garrett@xxxxxxxxxx>
- To: nanomsg@xxxxxxxxxxxxx, jimmy frasche <soapboxcicero@xxxxxxxxx>
- Date: Tue, 22 Apr 2014 01:28:33 -0700
Thanks for your feedback, more below:
On April 21, 2014 at 8:01:32 PM, jimmy frasche (soapboxcicero@xxxxxxxxx) wrote:
If you're not aware TestBusTLS fails and when tested with -race,
TestBusIPC and TestBusTCP fail as well, though not always and without
detecting any races.
Hmmm. You’re right. I don’t know *how* committed these without checking that
before. My guess is that Race is causing more dropped packets. In the case
of TestBus, what’s probably happening is that we are hitting backpressure, and
the message gets dropped. Without -race, the code is fast enough to keep up
without dropping messages. (I could probably try to change the code to reduce
the number of concurrent senders for TestBus — I wonder if I can tell if -race
is in use…)
So there’s no race, we’re just dropping messages due to perf. And the test
suite is intentionally picky about that.
From a cursory look:
Use golint: https://github.com/golang/lint
Yep. It doesn’t find anything (it did before). Is there something you think
it should report, but doesn’t?
—
RemEndpoint should be RemoveEndpoint.
I guess that’s a style nit. I’ll go ahead and make the change.
Is there any reason all the sps are in one package? It looks like
you've built a generic framework for them to implement. It would be
more idiomatic if you had something like
import "bitbucket.org/gdamore/mangos/req"
It would have been a nice idea, and it is actually what I wanted to do (for the
transports too!) But I had trouble making this work as well as I’d like,
because of circular includes. In particular, I want to register all the
*stock* protocols so that users don’t have to register them explicitly. The
problem I had with this is that:
mangos/req would depend on mangos
but mangos can’t register req in the core, because if it tries to import
mangos/req, circular imports and death.
...
r, err := req.Dial(...)
instead of
import "bitbucket.org/gdamore/mangos"
...
r, err := mangos.NewSocket(...)
A mangos.Dial could still work if the protocols register with mangos on init.
Hmm… so “init” in the package happens at import time? That could work, but it
would mean that end users would need to import each protocol or transport
specifically. (See below about my idea for another “all” place holder for
lazy developers who want everything. :-)
Additionally this would mean you could have a websocket transport
without requiring every one importing the package having to link to a
websocket package
Hmm… so you’re proposing that the application basically encode the knowledge.
I guess that would work ok, and as you notice it would shave off the big
imports (a real win for TLS and websocket!), but I’m thinking that it makes it
hard to write a “generic” app like nanocat, but maybe not much.
Sadly, I don’t see how I can apply this strategy to the transport layer, where
it would really have the most valuable impact, since the transport names are
encoded in user supplied strings.
Maybe I could use some intermediate package that contains a registry of all
types, and apps could just do “RegisterAllTransports” in the package or
something.
Let me think about this some more.
—
Why have ProtocolFactory be an interface instead of
type ProtocolFactory func() Protocol
Is there every any reason for a protocol factory to have any state
between calls? It doesn't seem like it should ever.
It doesn’t. I should change this code. I wrote it before I started playing
with function pointers in Go… interfaces seemed natural. I want to review this
again to see if it can be refactored more cleanly using closures.
—
The uint16 for protocol numbers could be a type with a String() method.
That’s a *neat* idea. The split of string to number made more sense when I had
separate protocol types for X vs normal (XREP, REP, etc.) implementations.
There are still the “X” names, but those could reasonably go away now that I’m
using a socket optoin to set raw mode.
—
You may want to consider implementing io.Reader/Writer for sockets.
The gorilla websocket package has a nice example for using it over a
message oriented protocol:
http://godoc.org/github.com/gorilla/websocket#Conn.NextReader
I will look at it, but I actually considered implementing more like a net.Conn
interface, and I decided that because of the semantic differences I gave up —
this is really un-streamlike.
Looking at the reference you gave though, it seems they just return an
io.Reader for the *message*. There are some weird semantics here though… you
Close() the io.Writer to send the message, you can’t have parallel accessors,
and the underlying connection remains open. This feels really kind of
contrived to get interface compatibility with things that want io.Reader or
io.Writer. Wouldn’t it just be easier to just use bytes.Buffer on the
msg.Body? Maybe the one useful addition would be send-on-close semantic.
(It doesn’t look it would be very hard to do this kind of interface, I just
never considered that it would be particularly useful.)
This allows a great deal of interop.
Can you point me at any examples of this?
- Garrett
Other related posts:
