- Fix for updating delivery subject and adjusting next delivery sequences.
- When acking explicitly but out of order, need to make sure we set floor correctly.
- Only update ack floors on an ack if the message is present.
- Fix for needAck for explicitAck out of order consumers detecting if message has been acked.
- Fix for race not locking stream when checking interest during stop.
- Fix for filestore determing if a message block still has a message. Added check to first sequence as well as cache.
- Some additions to the original test.
Signed-off-by: Derek Collison <derek@nats.io>
The test shows the issue.
It seems that Consumer.needAck() for AckExplicit should consider
that an Ack is needed if sseq > o.asflr and there is no pending
ack at all. However making this change would break the test
TestJetStreamInterestRetentionStream.
Also, running the new test with `-count 100` and by adding an
artificial delay in stream.ackMsg() (just before calling
mset.store.RemoveMsg(seq)) causes sometimes delete requests
for the same sequence to be processed twice, which causes
the new test to fail (even with an attempted fix as discussed
above). I think that the attempt to remove the same sequence twice
is messing up the state.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
The panic was caused by the closing of an already closed Go channel.
The Delete() relied on the consumer's mset being nil to consider
the consumer already closed. However, the consumer's mset is set
to nil after invoking sendDeleteAdvisoryLocked() which internally
invokes sendAdvisory() which releases/reacquires the consumer lock.
This left an open door for a race to occur and Delete() to be
invoked twice on the same consumer.
Moving setting the consumer's mset to nil too early would prevent
the sendAdvisory() to actually do its job. We could pass the mset
to sendAvisory(), but a simpler approach is to simply use a "closed"
boolean on the Consumer object that is set to true at the beginning
of the Delete() function.
Resolves#1621
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
It seems that when updating the delivery subject, we use as the
first sequence the ack floor, but if no message was ever ack'ed
then it causes the first message to be redelivered twice.
Resolves#1619
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
If the soliciting side detects the disconnect and attempts to
reconnect but the accepting side did not yet close the connection,
a "loop detected" error would be reported and the soliciting server
would not try to reconnect for 30 seconds.
Made a change so that the accepting server checks for existing
leafnode connection for the same server and same account, and if
it is found, close the "old" connection so it is replaced by
the "new" one.
Resolves#1606
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
This causes nil messages mid content stream and is confusing,
the test for this is amended to test the content received for
validity - where previously the test assumed any content is
good content but in fact it only received half the content.
Removing this behaviour until we can design this properly
Signed-off-by: R.I.Pienaar <rip@devco.net>
Users and NKey users will now have the option to specify a list
of allowed connection types.
This will allow for instance a certain user to be allowed to
connect as a standard NATS client, but not as Websocket, or
vice-versa.
This also fixes the websocket auth override. Indeed, with
the original behavior, the websocket users would have been bound
to $G, which would not work when there are accounts defined, since
when that is the case, no app can connect/bind to $G account.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Latency reports will include the header(s) responsible for the trace
Updated ADR to have it reflect implementation
Signed-off-by: Matthias Hanel <mh@synadia.com>
Made changes to processSub() to accept subscription properties,
including the icb callback so that it is set prior to add the
subscription to the account's sublist, which prevent races.
Fixed some other racy conditions, notably in addServiceImportSub()
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
The check that an account has to be signed by a configured operator is
done after fetch as well. As a consequence an account claim will never
become an Account in memory.
The original check during client or leaf authentication is left in
place.
Adding unit tests.
Modifying existing tests to not rely on an account but it's name instead.
Signed-off-by: Matthias Hanel <mh@synadia.com>
Previously unlimited accounts - ones who inherit server values - would
be unable to publish any messags at all
Signed-off-by: R.I.Pienaar <rip@devco.net>
If some servers in the cluster have the same connect URLs (due
to the use of client advertise), then it would be possible to
have a server sends the connect_urls INFO update to clients with
missing URLs.
Resolves#1515
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
That is, if the server receives "SUB foo 1" more than once from
the same client, we would register in the client map this subscription
only once, and add to the account's sublist only once, however we
would have updated shadow subscriptions and route/gateway maps for
each SUB protocol, which would result in inability to send unsubscribe
to routes when the client goes away or unsubscribes.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Leafnodes that formed clusters were partially supported. This adds proper support for origin cluster, subscription suppression and data message no echo for the origin cluster.
Signed-off-by: Derek Collison <derek@nats.io>
When a leafnode would connect with credentials that had permissions the spoke did not have a way of knowing what those were.
This could lead to being disconnected when sending subscriptions or messages to the hub which were not allowed.
Signed-off-by: Derek Collison <derek@nats.io>
Added cluster names as required for prep work for clustered JetStream. System can dynamically pick a cluster name and settle on one even in large clusters.
Signed-off-by: Derek Collison <derek@nats.io>
