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

JetStream pull over export service response type stream #6172

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

roeschter
Copy link

Suggested fix - no unit tests provided yet

When doing a Jetstream fetch request to a stream across an account boundary we export:
{ service: "$JS.API.>", accounts: [ "B" ], response_type: "stream" }

When the reply stream contain a nil message (no body no headers) the import reply subject mapping is removed. This seems to be by design. To Jetstream this looks like a message loss and results in timeouts and retries. Note that not just the nil message is suppressed but all messages coming after the nil messages are not delivered either.

Adding a header to all messages (e.g. Nats-Msg-Id) prevents this effect.

The fix here make removeRespServiceImport() Jetstream aware. Not removing it for messages which which $JS.ACK set. This is a minor fix with no performance impact as the check for JS reply subjects is done anyway further up in the code.

Also see internal Zendesk ticket [986]

Signed-off-by:m Michael Roeschter michael@synadia.com

@roeschter roeschter requested a review from a team as a code owner November 25, 2024 14:11
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

Need tests.

@derekcollison
Copy link
Member

Also this trick would not work for ack none IIRC since no reply would be given.

@wallyqs wallyqs changed the title Jetsstream pull over export service response type stream JetStream pull over export service response type stream Nov 26, 2024
@roeschter
Copy link
Author

Added two tests.

  1. TestMultiRequestReplyExportImportWithEmptyMessages - Tests that reply subject mapping for regular messages correctly breaks on nil message
  2. TestJetStreamFetchAcrossAccountExportImportWithEmptyMessages - Tests that reply subject mapping does NOT break for JS message and therefore fetch works correctly

@roeschter
Copy link
Author

roeschter commented Nov 26, 2024

Also this trick would not work for ack none IIRC since no reply would be given.

I just tested it an even with AckPolicy None, $JS.ACK is present

@roeschter
Copy link
Author

Customer for the time being used the workaround/recommendation to always set a header in Jetstream messages.

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