(2.11) ADR-44: JetStream Dynamic Metadata #5857
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
…kage Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
|
This will only report the api version from the meta-leader correct? |
The versions will be reported from the one answering the create/update/info request. Which will be only the stream/consumer leader hosting the asset at that time. |
derekcollison
requested changes
Sep 4, 2024
server/jetstream_api.go
Outdated
| @@ -1478,7 +1478,7 @@ func (s *Server) jsStreamCreateRequest(sub *subscription, c *client, _ *Account, | |||
| resp.StreamInfo = &StreamInfo{ | |||
| Created: mset.createdTime(), | |||
| State: mset.state(), | |||
| Config: mset.config(), | |||
| Config: includeDynamicStreamAssetVersionMetadata(mset.config()), | |||
server/jetstream_versioning_test.go
Outdated
| @@ -177,6 +208,51 @@ func TestJetStreamSetConsumerAssetVersionMetadata(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestJetStreamSetConsumerAssetVersionMetadataRemoveDynamicFields(t *testing.T) { | |||
There was a problem hiding this comment.
Test names matter since they need to plug into the CI/CD matching when running the tests in parallel.
There was a problem hiding this comment.
The test names are currently prefixed with TestJetStream so they are run as part of the js_tests. This file also contains the following on top:
//go:build !skip_js_tests
// +build !skip_js_testsIs this sufficient or does something need to be changed?
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
server/jetstream_versioning.go
Outdated
| @@ -47,13 +53,29 @@ func setStaticStreamMetadata(cfg *StreamConfig, prevCfg *StreamConfig) { | |||
| cfg.Metadata[JSRequiredLevelMetadataKey] = strconv.Itoa(requiredApiLevel) | |||
| } | |||
|
|
|||
| // setDynamicStreamMetadata adds dynamic fields into the (copied) metadata. | |||
| func setDynamicStreamMetadata(cfg StreamConfig) StreamConfig { | |||
There was a problem hiding this comment.
Doing the pass by value / copy here twice again, in and out of this function.
There was a problem hiding this comment.
Changed to use pointers
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
derekcollison
pushed a commit
that referenced
this pull request
Sep 7, 2024
Implements [ADR-44](https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-44.md) Initial addition of JetStream Asset Versioning, containing: - generating stream and consumer metadata - created server version: `_nats.created.server.version` - created server API level: `_nats.created.server.api_level` - required server API level: `_nats.server.require.api_level` - logging supported API level upon startup: `[INF] API Level: 1` <br> Note that stream and consumer metadata is only set/updated upon: - creating a new stream/consumer - updating a stream/consumer _(created metadata will not be set for pre-existing assets, only required level will be updated)_ <br> Many tests are added ensuring that: - restoring streams from backups doesn't set this metadata _(if it doesn't exist, for example restoring a backup from a previous version)_ - restarting a server (or upgrading) doesn't set this metadata _(if it doesn't exist, for example due to upgrading)_ - updating consumer `PauseUntil` ups or lowers required API level - metadata is consistently reported through add, update and info requests - only stream/consumer/meta leader determine the metadata, so that there can be no skew in metadata if followers were allowed to update as well - users can't manually supply these `_nats.>` metadata fields, they will be overwritten to the appropriate values - if the metadata is missing during an update, the metadata is preserved _(for example an update by a client that doesn't know about metadata yet)_ These tests check for both non-clustered R1 setups and clustered R3 setups. <br> This PR doesn't fully implement [ADR-44](https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-44.md). There will be follow-up PRs later on to add support for: - dynamic metadata, like `_nats.server.version` and `_nats.server.api_level` that report the current server version and API level the asset lives on: #5857 - reporting server JS API level through `jsz`, `varz` and `$JS.API.INFO` (it is already reported in the logs during startup): #5855 - etc. There are some slight inconsistencies between the ADR and this PR. For example using the terms feature level, `api_version` and `api_level` that all mean the same and are all made consistent to be `api_level`. I'll correct the ADR with any changed details after this PR has been merged. Signed-off-by: Maurice van Veen <github@mauricevanveen.com> --------- Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements ADR-44
Extends #5850
Add dynamic JetStream stream/consumer metadata:
These are not persisted and are only returned in the response of a create/update/info request.
Signed-off-by: Maurice van Veen github@mauricevanveen.com