There is a mechanism to detect if a connection somewhere in the
cluster is using the session ID of an existing one, and if so,
close one as a duplicate.
However, when different domains are used, they should not be considered
duplicates.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
When there was a failure to delete a QoS1 consumer, the session
would still be deleted, which would cause orphaned consumers.
In case of error, the session record will not be deleted, which means
that it is still possible to restart the session and then close
it (with the clean flag).
Relates to #3116
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Ability to override the stream and consumers replica count, which is by default
determined based on the cluster size.
```
mqtt {
port: 1883
stream_replicas: 5
consumer_replicas: 1
}
```
The above would allow *new* MQTT streams to be created with a replicas
factor of 5 (it will be an error if the cluster does not have that
many nodes, and error will occur at runtime when the first client
on a given account connects), and new consumers would be R=1.
The MQTT existing streams/consumers for an account are not modified.
The stream_replicas can also obviously be reduced to 1 for a cluster
of 3 nodes if one desire to have those streams as R=1.
A value of 0 or negative is considered letting the server pick
the value (from 1 to 3 depending on standalone/cluster size).
There is another property that allows the consumers to be created
with memory storage instead of file:
```
mqtt {
..
consumer_memory_storage: true
}
```
Those new settings are global and apply to new streams/consumers
only.
Related to #3116
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Update warning
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Removed the warnings, instead have a sync.Map where they are
registered/unregistered and can be inspected with an undocumented
monitor page.
Added the notion of "in progress" which is the number of messages
that have beend pop()'ed. When recycle() is invoked this count
goes down.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Along a leaf node connection, unless the system account is shared AND the JetStream domain name is identical, the default JetStream traffic (without a domain set) will be denied.
As a consequence, all clients that wants to access a domain that is not the one in the server they are connected to, a domain name must be specified.
Affected from this change are setups where: a leaf node had no local JetStream OR the server the leaf node connected to had no local JetStream.
One of the two accounts that are connected via a leaf node remote, must have no JetStream enabled.
The side that does not have JetStream enabled, will loose JetStream access and it's clients must set `nats.Domain` manually.
For workarounds on how to restore the old behavior, look at:
https://github.com/nats-io/nats-server/pull/2693#issuecomment-996212582
New config values added:
`default_js_domain` is a mapping from account to domain, settable when JetStream is not enabled in an account.
`extension_hint` are hints for non clustered server to start in clustered mode (and be usable to extend)
`js_domain` is a way to set the JetStream domain to use for mqtt.
Signed-off-by: Matthias Hanel <mh@synadia.com>
When a consumer is configured with "meta-only" option, and the
stream was backed by a memory store, a memory corruption could
happen causing the application to receive corrupted headers.
Also replaced most of usage of `append(a[:0:0], a...)` to make
copies. This was based on this wiki:
https://github.com/go101/go101/wiki/How-to-efficiently-clone-a-slice%3F
But since Go 1.15, it is actually faster to call make+copy instead.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
This way, any log statement for a client will include the client id,
similar to how the server now logs information about NATS clients
(such as language, version, connection name).
Also adding a debug statement once the client has successfully connected.
Here is how this will look like for a client with client id "client_0".
```
[69591] 2021/10/06 10:06:50.837977 [DBG] [::1]:57415 - mid:18 - Client connection created
[69591] 2021/10/06 10:06:50.839871 [DBG] [::1]:57415 - mid:18 - "client_0" - Client connected
[69591] 2021/10/06 10:07:00.627307 [DBG] [::1]:57415 - mid:18 - "client_0" - Client connection closed: Client Closed
```
All log statements will be affected, for instance here is an auth error:
```
[69591] 2021/10/06 10:09:48.618964 [DBG] [::1]:57424 - mid:23 - Client connection created
[69591] 2021/10/06 10:09:48.619015 [ERR] [::1]:57424 - mid:23 - "client_0" - authentication error - User "mqtt"
[69591] 2021/10/06 10:09:48.619026 [DBG] [::1]:57424 - mid:23 - "client_0" - Client connection closed: Authentication Failure
[69591] 2021/10/06 10:09:48.619038 [ERR] [::1]:57424 - mid:23 - "client_0" - unable to connect: authentication error
```
Resolves#2587
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
When a session record exists and is currently not connected, if
the user reconnects with the same client ID but with now the
clean flag set, we are required to clear the state. In earlier
implementation (where we were using a stream per session), we
were not deleting the stream to be created right away. Since now
we just have a message per session, it is ok to delete that
message when clearing the existing session that is going to be
replaced.
Also made apiDispatch execute in place for any connection that
is not ROUTER or GATEWAY.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
ClientID has been added to various monitoring objects. Also, added
the ability to filter connections on `client_id`.
On auth violation, the proper code was not invoked, which meant
that no disconnect event (with auth reason) would be published.
Resolves#2270
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
With the availability of a "max message per subject" for a given
stream, it is possible to replace individual streams that were
created per session with a single stream that gets all sessions
as a single message per subject, which subject is composed of
the session client ID hash.
The first time the new stream is created for a given account,
all existing MQTT session streams will be transferred to the
new mux'ed MQTT session stream.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Previously we had a few confusing functions like NewT
and similar that were quite fragile to use due to minimal
validation and a panic in go stdlib string Replacer.
Now we generate helper methods for every string, these
are used to access errors, fill in templates and conditional
returns of error type using the new Unless() option
We now get compile time errors for some common mistakes
and have better IDE helpers for arguments etc
Signed-off-by: R.I.Pienaar <rip@devco.net>
When processing service imports we would swap out the accounts during processing.
With the addition of internal subscriptions and internal clients publishing in JetStream we had an issue with the wrong account being used.
This was specific to delyaed pull subscribers trying to unsubscribe due to max of 1 while other JetStream API calls were running concurrently.
The issue was that the subscription created for the MQTT client
was resulting in creation of a shadow subscription which did not
have the mqtt specific object attached, which would cause the
panic when accessing it in the sub's icb.
After that, it was discovered that the wrong subject was passed
to deliverMsg(), so fixed that too so that the icb callback gets
the proper transformed subject.
Resolves#2265
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
* [changed] pinned certs to check the server connected to as well
on reload clients with removed pinned certs will be disconnected.
The check happens only on tls handshake now.
Signed-off-by: Matthias Hanel <mh@synadia.com>
* [added] pinned_cert option to tls block hex(sha256(spki))
When read form config, the values are automatically lower cased.
The check when seeing the values programmatically requires
lower case to avoid having to alter the map at this point.
Signed-off-by: Matthias Hanel <mh@synadia.com>
Say with a cluster of 3, all MQTT assets are created with a replicas
of 3. However, when a server is shutdown, then any new MQTT client
will fail to connect because we try to create a session stream
with R(3), which leads to insufficient resources.
The longer term solution should be for the server to allow the
creation of an asset with a R() value that is bigger than the
current number of running servers as long as there is quorum.
For now, we will reduce the R() value for the sessions if we get
an "insufficient resources" error.
Note that the other assets still will use the compute R() based
on cluster size. So the first time that a client on a given
account is started, we will still need to have R() == cluster size
(at least for R(3)).
Partially resolves#2226
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
At the time where we need the domain to construct the session hash,
we have access to server options. So use that instead of storing
the domain in the internal mqtt structure.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
This will solve the issue of naming the durable per server for
the "retained messages" stream in situation where a cluster
of servers would not have JetStream defined but connect to another
cluster that has it. All the servers within the cluster without
JetStream would cause the durable's delivery subject to be updated
to the last server starting the durable.
Updated the check for mqtt requiring JetStream if running in
standalone mode to check that no leafnode configuration is present.
Replaced use of fmt.Errorf() when the string was static with
errors created with errors.New(). Updated tests that were checking
for errors to use those errors instead of repeating the string.
Added test that has a hub cluster with JS enabled and a remote server
that has mqtt{} without JetStream and ensure that this works.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Some issues that have been fixed would manifest by timeouts on
connect, unexpected memory usage on high publish message rate.
Some details:
- Replies were not always GW routed properly because we were looking
at the wrong connection's rsubs
- GW routed replies would not be found because they were tracked
in the subscription's client object, which may not be the same used
to send the reply
- Increased the mqtt timeout to wait for JS replies since in some
tests it was sometimes taking more than the original 2 seconds
- Incoming gateway messages destined for an MQTT internal subscription
may have been rejected as a no interest if the account had service imports
- Don't use time.After(), instead create explicit timer so it can
be stopped when not timing out.
- Unnecessary copy of a slice since we were converting to a string anyway.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
In cluster mode, a sub connects to server 1, another on server 2.
A publisher connects to server 2 and publishes a retained message.
If both subs restart they would properly receive the retained message.
However, if the publisher sens an empty message that "removes" the
retained message for this topic, and then consumer that connects to
server 1 restarts, it would not receive the retained message as it
should.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
In a setup with shared system account and a cluster of leaf nodes,
the JS requests did not contain the origin cluster, which caused
assets to possibly be created in the HUB. With this change, the
assets will be created in the origin cluster.
Also, removed use of acc.JetStreamEnabled() but instead fail
start of the server if mqtt is enabled in standalone mode and JS
is not enabled. If JS is enabled, we will get proper error if
account has no JS enabled.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Websocket is currently not supported for MQTT clients. When a
client tries to connect with websocket protocol to the MQTT port,
the error message: `mid:9 - not connected` would be logged, which
is not really telling.
The server will now guess if the connection was websocket and report
a more appropriate error message, such as:
```
invalid connection, websocket currently not supported
```
Resolves#2126
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Currently, we use ReadyForConnections in server tests to wait for the
server to be ready. However, when this fails we don't get a clue about
why it failed.
This change adds a new unexported method called readyForConnections that
returns an error describing which check failed. The exported
ReadyForConnections version works exactly as before. The unexported
version gets used in internal tests only.
In error conditions or when replacing an existing sub qos1 to qos0,
we were unsubscribing the NATS subscription, but that would not
have been propagated across the cluster.
Also fixed a flapper
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
This also applies to times that end up in that json.
Where applicable moved time.Now() to where it is used.
Moved calls to .UTC() to where time is created it that time is converted
later anyway.
Signed-off-by: Matthias Hanel <mh@synadia.com>
- Keep track of published topic to avoid conversion in some case
- Rework tracing of some MQTT protocols
- Rework topic-to-subject conversion
- Update some tests
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
The test TestMQTTPersistedSession() flapped once on GA. It turns
out that when the server was sending CONNACK the test was immediately
using a NATS publisher to send a message that was not received by
the MQTT subscription for the recovered session.
Sending the CONNACK before restoring subscriptions allowed for a
window where a different connection could publish and messages
would be missed. It is technically ok, I think, and test could
have been easily fixed to ensure that we don't NATS publish before
the session is fully restored.
However, I have changed the order to first restore subscriptions
then send the CONNACK. The way locking happens with MQTT subscriptions
we are sure that the CONNACK will be sent first because even if
there are inflight messages, the MQTT callbacks will have to wait
for the session lock under which the subscriptions are restored
and the CONNACK sent.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Based on how the MQTT callback operates, it is safe to finish setup
of the MQTT subscriptions after processSub() returns. So I have
reverted the changes to processSub() which will minimize changes
to non-MQTT related code.
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
