[FIX] remove extra system headers on direct gets #4618
Conversation
We would have to scan at least once, but a better question is perhaps we should be doing this on the on-board? The issue is republishing is causing a recapture by the stream. If the stream is guaranteed not to have the header, then no cleanup would be necessary. |
aricart
force-pushed
the
fix-4573
branch
2 times, most recently
from
2916bb0
to
36b4e2d
Compare
|
@derekcollison discussed with @neilalexander an alternative strategy - remove these headers before onboarding into the stream. That keeps direct as fast as possible and prevents serialization of any |
|
(and yes, I added one function to remove all in one shot) |
|
Made a change to constrain the actual headers that are removed as some are handled further down during the Store() operation. |
| @@ -4399,6 +4400,11 @@ func (mset *stream) processJetStreamMsg(subject, reply string, hdr, msg []byte, | |||
| } | |||
| } | |||
|
|
|||
| // if we are NOT doing subject remapping, this is a message to on-board it shouldn't store Nats-* headers | |||
| if tsubj == _EMPTY_ { | |||
| hdr = removeStreamIdentityHeaders(hdr) | |||
There was a problem hiding this comment.
More importantly - this doesn't fix the issue for direct get - it simply solves it for the on-boarding. We would still need to have the initial fix on the outbound to ensure that clients won't get duplicate headers.
There was a problem hiding this comment.
Yes since they are stored with headers today..
… it simply appends JSON bytes to existing headers. If the message was onboarded on a republish, this will contain system headers. Since headers are not ordered and can contain multiple values for the same header, this can break KV clients as well as create ambiguity on the stream that yielded the message. Fix #4573
|
@derekcollison this is possibly the correct fix, if we think that we want to reduce the surface area, we can keep the on-boarding of the the stream identity headers, but this is the best we can do. The direct API checks if there's a |
|
I am conflicted, I think we need to retain what the app sent us. |
[FIX] direct get APIs can contain duplicate
Nats-Stream,Nats-Sequence,Nats-Time-StampandNats-Subjectheaders because it simply appends JSON bytes to existing headers. If the message was onboarded on a republish, this will contain the above system headers. Since headers are not ordered and can contain multiple values for the same header, this can break KV clients and create ambiguity on the stream that yielded the message.Resolves #NNNgit pull --rebase origin main)Resolves #4573
Changes proposed in this pull request:
Nats-Stream,Nats-Sequence,Nats-Time-StampandNats-Subjectthat may be present on the stream message when using direct get apis