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

[IMPROVED] MQTT: do not JSON-encode retained message bodies #4825

Merged
merged 3 commits into from
Nov 30, 2023

Conversation

levb
Copy link
Contributor

@levb levb commented Nov 28, 2023

BREAKING

This change is not backwards-compatible. It will break in a mixed environment and/or when downgrading.

if a server runs this code, and the cluster/server is reverted to an older version or there are servers in the cluster that are still running older versions, the retained messages will fail to be loaded.

SUMMARY

We used to JSON-encode the payloads of retained messages when storing to JetStream. This PR moves the JSON metadata into the NATS header, and stores the message bytes unencoded in the NATS message body.

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM, but let's be clear (for server release process purposes), that if a server runs this code, and the cluster/server is reverted to an older version, the retained messages will fail to be loaded.

@levb levb merged commit e0d45bd into main Nov 30, 2023
@levb levb deleted the lev-donot-json-ret-msg-body branch November 30, 2023 12:08
neilalexander pushed a commit that referenced this pull request Nov 30, 2023
#### BREAKING

This change is not backwards-compatible. It will break in a mixed
environment and/or when downgrading.

if a server runs this code, and the cluster/server is reverted to an
older version or there are servers in the cluster that are still running
older versions, the retained messages will fail to be loaded.

#### SUMMARY

We used to JSON-encode the payloads of retained messages when storing to
JetStream. This PR moves the JSON metadata into the NATS header, and
stores the message bytes unencoded in the NATS message body.
levb added a commit that referenced this pull request Nov 30, 2023
@levb levb mentioned this pull request Nov 30, 2023
levb added a commit that referenced this pull request Nov 30, 2023
Revert #4825. It had at least
one significant issue that needs to be fixed.

`git revert e0d45bd` was clean
nabsul pushed a commit to nabsul/nats-server that referenced this pull request Jan 15, 2024
…4825)

This change is not backwards-compatible. It will break in a mixed
environment and/or when downgrading.

if a server runs this code, and the cluster/server is reverted to an
older version or there are servers in the cluster that are still running
older versions, the retained messages will fail to be loaded.

We used to JSON-encode the payloads of retained messages when storing to
JetStream. This PR moves the JSON metadata into the NATS header, and
stores the message bytes unencoded in the NATS message body.
nabsul pushed a commit to nabsul/nats-server that referenced this pull request Jan 15, 2024
Revert nats-io#4825. It had at least
one significant issue that needs to be fixed.

`git revert e0d45bd` was clean
derekcollison added a commit that referenced this pull request Feb 13, 2024
…4825) (#5050)

This change is for 2.11 only. As noted in the original PR, downgrading
to 2.10 and below would result in data loss for MQTT retained messages
(and potential error handling issues).

#4825 was reverted from main,
adding it back now, with some changes. Specifically, no longer use JSON
for the header value; instead use 4 separate headers.

The revert was messy, not sure I can reproduce the issue seen in the
original PR.
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.

2 participants