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

Cherry picks for v2.10.19 RC5 #5823

Merged
merged 17 commits into from
Aug 23, 2024
Merged

Cherry picks for v2.10.19 RC5 #5823

merged 17 commits into from
Aug 23, 2024

Conversation

@wallyqs wallyqs requested a review from a team as a code owner August 23, 2024 15:11
derekcollison and others added 16 commits August 23, 2024 09:30
…here it is faster.

Also using the new (Go 1.21) cmp.Compare in the sort functions.
Also fixes a variable shadowing that somehow wasn't being detected when using `sort.Slices` on that shadowing variable.

Signed-off-by: Jean-Noël Moyne <[email protected]>
Signed-off-by: Jean-Noël Moyne <[email protected]>
Signed-off-by: Waldemar Quevedo <[email protected]>
The check of the allowed connection types must be maded after
the user's permissions limits are possibly "swapped" with the
scoped signer's.

Resolves #5786

Signed-off-by: Ivan Kozlovic <[email protected]>
…out` on monitoring HTTP server

Reported-by: Trail of Bits <https://www.trailofbits.com>
Signed-off-by: Neil Twigg <[email protected]>
…s every time (i.e. if it includes something like a count or a number that changes with every log line)

Added rateLimitFormatWarnf that does the rate limiting for all log messages according to the format (rather than the statement).
Add rateLimitFormatWarnf to the test
The `getExpectedLastSeqPerSubject` check in `processJetStreamMsg` would take into account that we could get a create/seq=0 for last seq per subject.
However, the `processClusteredInboundMsg` would NOT take this into account. This would result in sending the message to all replicas. If any replica would store instead of reject the message, it would desync.

Signed-off-by: Maurice van Veen <[email protected]>
Upon graceful shutdown, stream snapshots aren't installed:
stream.go
```go
} else {
	// Always attempt snapshot on clean exit.
	n.InstallSnapshot(mset.stateSnapshotLocked())
	n.Stop()
}
```

But raft.go exits immediately since it's in Closed state
```go
if n.State() == Closed {
        return errNodeClosed
}
```

This turns out to be because of calling either of `close(mset.mqch)` or
`close(mset.qch)`. Which then calls the following upon leaving
`monitorStream`:
```go
	// Make sure to stop the raft group on exit to prevent accidental memory bloat.
	// This should be below the checkInMonitor call though to avoid stopping it out
	// from underneath the one that is running since it will be the same raft node.
	defer n.Stop()
```

This PR proposes to skip running `n.Stop()` from `monitorStream` when we
know we're closing / have called `mset.stop()`. Which will already take
care of calling either `n.Stop()` or `n.Delete()` so there's no need in
stopping from `monitorStream` in that case.

TLDR; this ensures stream snapshots are installed upon shutdown.

Signed-off-by: Maurice van Veen <[email protected]>

---------

Signed-off-by: Maurice van Veen <[email protected]>
Use a dedicated internal receive queue to dispatch the StatsZ/Ping
events, and have Profilez run in its own go routine (not even
using internal receive queues) because execution of profilez()
can take several seconds.

Signed-off-by: Ivan Kozlovic <[email protected]>
…ip on initial miss in LoadNextMsg().

When we would miss a LoadNextMsg() from the block associated with the starting sequence, we would check if we could skip ahead.
The logic for this could load old blocks, behind where we were, if the psim fblk was outdated, causing memory bloat for cache expiration time intervals.

We reworked this to load no blocks in the skip check since this is hot path for LoadNextMsg() and the linear scan, if applicable, properly will expire anything loaded that did not contain a match.

Signed-off-by: Derek Collison <[email protected]>
Some tests were showing often a race between updateWithAdvisory()
and isFiltered().

There may be other data races in other files that read the mset's
configuration without proper locking, but this PR addresses
specifically issues between consumer (consumer.go) and stream.

Signed-off-by: Ivan Kozlovic <[email protected]>
Signed-off-by: Ivan Kozlovic <[email protected]>
@wallyqs wallyqs force-pushed the downstream/v2.10.19 branch from 9ad2116 to 1dc8f39 Compare August 23, 2024 16:34
@derekcollison
Copy link
Member

Let's pull in Maurice's PR too here.

A duplicate message inserted into an Interest-based stream with no
interest would return 0 as sequence instead of the sequence of the
initial message.

```sh
$ nats req interest '' -H 'Nats-Msg-Id: dedupe'
20:31:43 Sending request on "interest"
20:31:43 Received with rtt 187.55µs
{"stream":"INTEREST", "seq":6}

$ nats req interest '' -H 'Nats-Msg-Id: dedupe'
20:31:43 Sending request on "interest"
20:31:43 Received with rtt 101.95µs
{"stream":"INTEREST", "seq":0,"duplicate": true}
# ---- seq should be 6 -----^
```

Signed-off-by: Maurice van Veen <[email protected]>

---------

Signed-off-by: Maurice van Veen <[email protected]>
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.

LGTM

@wallyqs wallyqs merged commit 26c8196 into release/v2.10.19 Aug 23, 2024
5 checks passed
@wallyqs wallyqs deleted the downstream/v2.10.19 branch August 23, 2024 19:54
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.

6 participants