This PR is based out of #633. It imroves parsing QRSID so that the
TestRouteQueueSemantics test now passes (when dealing with malformed
QRSID).
A test similar to what is reported in #632 was also added. This
test however, uncovers a race condition that will be fixed in a
separate PR.
Resolves#632
This is the result of flapping tests in go-nats that were caused
by a defect (see PR https://github.com/nats-io/go-nats/pull/348).
However, during debugging, I realize that there were also things
that were not quite right in the server side. This change should
make it the notification of cluster topology changes to clients
more robust.
Until now, a server would only notify clients of servers that join
the cluster. More than that, a server would send ot its clients only
information if new servers were added.
This PR changes this by sending to clients that support async INFO
the list of URLs for all servers in the cluster any time that there
is a change (joining or leaving the cluster).
As of now, clients will not be affected by the change (and will not
take benefit of this: removing servers from their server pool). This
will be addressed in each supported client once this is merged.
If server requires TLS and clients are connecting, and a Connz
request is made while clients are still in TLS Handshake, the
call to tls.Conn.ConnectionState() would block for the duration
of the handshake. This would cause the overall http request to
take too long.
We will now not try to gather TLSVersion and TLSCipher from a
client that is still in TLS handshake.
Resolves#600
- Created a setter for the closed flag.
- Check if route is closed under lock and set a boolean if so,
so we don't check c.route outside of c's mutex.
- Ensure that we do not create a route on shutdown, which would
leave a connection hanging (was seen in some config reload tests).
gnatsd currently uses a global logger. This can cause some problems
(especially around the config-reload work), but global variables are
also just an anti-pattern in general. The current behavior is
particularly surprising because the global logger is configured through
calls to the Server.
This addresses issue #500 by removing the global logger and making it a
field on Server.
- Removed unnecessary cast check to (*net.TCPConn). When the timer
fires, the connection is already established. Replaced with check
that connection has not been closed.
- Add PING test that checks that pings are sent to TLS connections.
- Changed Go version to 1.7.5 in travis.
- Removed test package from code coverage.
This happens sometimes, and the latest occurence was today:
https://github.com/nats-io/java-nats/issues/96
When it happens, there is no error but subscribers would not receive
anything, etc...
This PR uses the fact that clients set the field Lang in the CONNECT
protocol that ROUTEs do not. I have checked that all Apcera supported
clients do set Lang in the CONNECT protocol.
If we plan to add Lang for routes, we need to find another field or
use a new one, in which case that would work only for new clients
(that would need to be updated).
With this change, when the server accepts a connection on the route
port and detects that this protocol field is present, it now closes
the client connection.
The nice thing is that newer clients, when incorrectly connecting
to the route port, get from the route's INFO the list of client URLs,
which means that on the initial connect error, they are able to
subsequently connect to the proper client port, so it is transparent
to the user (which may or may not be a good thing). However, it is not
guaranteed because if the client is not setting NoRandomize to true,
the client URL is added but the array shuffled, so it is possible that
the client library does not find the correct port in the connect loop.
We use a hardcoded value of 2 seconds for Write deadline when
writing data to client's socket.
This PR makes that value configurable.
Question is should we push the setting down to the client's object
to avoid indirection such as client.srv.opts.WriteDeadline?
This will protect the server from non NATS clients (telnet, etc),
or misbehaving clients that would create the tcp connection but
block before sending the CONNECT.
The drawback is that the client may or may not receive the error
message (in my tests, it was getting only between 10%-20% of times).
Clients that will be at the ClientProtoInfo protocol level (or above)
will now receive an asynchronous INFO protocol when the server
they connect to adds a *new* route. This means that when the cluster
adds a new server, all clients in the cluster should now be notified
of this new addition.
This PR addresses the issue by testing the result of the
`Timer.Stop()` method, which if `false` indicates that the callback
may be in progress. When that happens, the `processConnect` method
will wait for the callback to finish and simply return. The auth
timeout error will therefore be received by the client during the
synchronous connect process, which will allow it to try again.
Resolves#289
This commit has 3 changes:
- Remove duplicate logic for creating cache results map.
- Move code out of an if that will always be true and remove the if.
- Use == 0 instead of <= 0 for comparing the return value of len(X) as the value can never be < 0.
