Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/mtm 58268/document mqtt service topic limiting #2464

Open
wants to merge 4 commits into
base: private-preview/mqtt-connect
Choose a base branch
from

Conversation

abor-c8y
Copy link

Additional sections outlining changes included in MQTT Service topic limiting feature.

Details regarding receiving errors for topic limit related failures for MQTT 3 clients via error topic are absent as this is yet to be implemented: https://github.com/Cumulocity-IoT/c8y-mqtt-service/pull/153

Copy link
Contributor

Preview available here

@@ -40,6 +40,29 @@ Other than that you are free to use any topic name which is compatible with the
Wildcard topics (`+`, `#`) and system topics starting with `$` are not supported.
{{< /c8y-admon-info >}}

#### Topic limit {#topic-limit}

MQTT Service has the ability to limit the total number of topics that a single tenant can create, current default is no limit.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MQTT Service has the ability to limit the total number of topics that a single tenant can create, current default is no limit.
MQTT Service has the ability to limit the total number of topics that a single tenant can create. The current default is no limit.

the delivery of the packet is prevented.

In the case of MQTT 5 clients have access to reason code and reason string describing the failure when using QoS 1 via acknowledgements,
reason code being QUOTA_EXCEEDED: 0x97.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
reason code being QUOTA_EXCEEDED: 0x97.
reason code being `QUOTA_EXCEEDED: 0x97`.


In the case of MQTT 5 clients have access to reason code and reason string describing the failure when using QoS 1 via acknowledgements,
reason code being QUOTA_EXCEEDED: 0x97.
In the case of MQTT 3.1 and 3.1.1 clients only have access to reason code describing the failure when using QoS 1 via acknowledgements and only
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In the case of MQTT 3.1 and 3.1.1 clients only have access to reason code describing the failure when using QoS 1 via acknowledgements and only
In the case of MQTT 3.1 and 3.1.1 clients only have access to the reason code describing the failure when using QoS 1 via acknowledgements and only

In the case of MQTT 5 clients have access to reason code and reason string describing the failure when using QoS 1 via acknowledgements,
reason code being QUOTA_EXCEEDED: 0x97.
In the case of MQTT 3.1 and 3.1.1 clients only have access to reason code describing the failure when using QoS 1 via acknowledgements and only
for SUBSCRIBE packets, where the reason code is 0x80.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for SUBSCRIBE packets, where the reason code is 0x80.
for the SUBSCRIBE packets, where the reason code is `0x80`.

reason code being QUOTA_EXCEEDED: 0x97.
In the case of MQTT 3.1 and 3.1.1 clients only have access to reason code describing the failure when using QoS 1 via acknowledgements and only
for SUBSCRIBE packets, where the reason code is 0x80.
For PUBLISH packets, client will be disconnected with no further information as per the MQTT specification.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For PUBLISH packets, client will be disconnected with no further information as per the MQTT specification.
For PUBLISH packets, the client will be disconnected with no further information as per the MQTT specification.

#### Topic cleanup {#topic-cleanup}

MQTT Service will automatically remove topics which are no longer in use, topics are recognized as inactive when there are no subscriptions and
internal publisher to the topic is closed. The publisher is responsible for publishing modified MQTT Service messages to the correct topic and these
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
internal publisher to the topic is closed. The publisher is responsible for publishing modified MQTT Service messages to the correct topic and these
the internal publisher to the topic is closed. The publisher is responsible for publishing the modified MQTT service messages to the correct topic. These

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative that adds this below. 'These sounded like the messages as it was the only plural entity in the previous sentence, so below I've changed 'These' to 'The publishers' to be clearer. I also improved the following sentence

@@ -1,4 +1,4 @@
---
title: Introduction
title: MQTT Service
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
title: MQTT Service
title: MQTT service

Copy link
Contributor

@gapa-c8y gapa-c8y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to go through these with you (it might be confusing given I've added to @MWindel's comments. Also I might need correction to my understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants