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

(2.11) ADR-44: JetStream API Level in varz,jsz,statsz,$JS.API.INFO #5855

Merged
merged 15 commits into from
Sep 7, 2024

Conversation

MauriceVanVeen
Copy link
Member

Implements ADR-44
Extends #5850

Expose the supported JetStream API level in varz, jsz, statsz and $JS.API.INFO. Both over the HTTP endpoints as well as requests to $SYS.REQ.SERVER.PING.VARZ and the like.

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

@MauriceVanVeen MauriceVanVeen requested a review from a team as a code owner September 2, 2024 14:23
Signed-off-by: Maurice van Veen <[email protected]>
@@ -86,6 +86,7 @@ type JetStreamTier struct {
type JetStreamAccountStats struct {
JetStreamTier // in case tiers are used, reflects totals with limits not set
Domain string `json:"domain,omitempty"`
APILevel string `json:"api_level"`
Copy link
Member

Choose a reason for hiding this comment

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

Should this be under API instead as just level?

Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't do it at first since API contains dynamic stats like total and errored requests, and the JSApiLevel is static/hardcoded just like the server VERSION is.

The fields have the same prefix and it actually simplifies the code as well when moving it in, though.
Assuming it's still okay, I've moved it into API.Level.

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

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 <[email protected]>

---------

Signed-off-by: Maurice van Veen <[email protected]>
Base automatically changed from adr-44/jetstream-asset-versioning to main September 7, 2024 01:18
@derekcollison derekcollison merged commit 2f6d8a2 into main Sep 7, 2024
5 checks passed
@derekcollison derekcollison deleted the adr-44/varz-jsz-acc-info branch September 7, 2024 18:46
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.

3 participants