Improve `jetstream` documentation #1532

piotrpio · 2024-01-24T16:32:05Z

This is the first part of improving jetstream package documentation.
This PR updates documantation for core jetstream (no KV or Object Store), by expanding docs to struct fields, as well as having more detailed documentation on public methods and interfaces.

What will be added in subsequent PRs:

KV/Object store documentation improvements, together with respective section in README
Code examples

Signed-off-by: Piotr Piotrowski [email protected]

Signed-off-by: Piotr Piotrowski <[email protected]>

Jarema

This is huge improvement for the docs!

Some comments added.

jetstream/consumer.go

Jarema · 2024-01-25T08:46:09Z

jetstream/consumer_config.go

-		OptStartTime      *time.Time    `json:"opt_start_time,omitempty"`
-		ReplayPolicy      ReplayPolicy  `json:"replay_policy"`
+		// FilterSubjects is a set of subjects that overlap with the subjects
+		// bound to the stream to filter delivered messages. Requires


same point on overlap as before.

jetstream/jetstream.go

Jarema · 2024-01-25T08:52:32Z

jetstream/jetstream.go

-		// Consumer interface is returned, serving as a hook to operate on a consumer (e.g. fetch messages)
+
+		// CreateConsumer creates a consumer on a given stream with given
+		// config. If consumer already exists, ErrConsumerExists is returned.


It will not return error if consumer already exists, but the config is exactly the same.

jetstream/stream_config.go

Co-authored-by: Tomasz Pietrek <[email protected]>

ripienaar

really great effort

jetstream/consumer.go

ripienaar · 2024-01-25T10:17:25Z

jetstream/consumer.go

 		FetchNoWait(batch int) (MessageBatch, error)
-		// Consume can be used to continuously receive messages and handle them with the provided callback function
+
+		// Consume can be used to continuously receive messages and handle them


Under what circumstances will it not be continuous? Similar question on the description of the struct itself and others mentioning continuous consumption.

I don't know this API so maybe asking dumb questions sorry

This is I think not worded properly. Consume and Messages are continuous and will never stop with exception of a few conditions:

user calls Stop/Drain

consumer deleted

consumer not found after server reconnect (after multiple retries)

That's true for standard pull consumer anyway, for ordered consumer "consumer deleted" is not terminal - we just create a new one. So I don't think this is something that I would put as a comment on the Consumer interface.

I will reword the descriptions to make sure it's clear that those methods are intentionally endless.

ripienaar · 2024-01-25T10:19:41Z

jetstream/consumer.go

-		// Next is used to retrieve the next message from the stream.
-		// This method will block until the message is retrieved or timeout is reached.
+
+		// Next is used to retrieve the next message from the stream. This


from the consumer... I know its technically fetching from the stream but saying its from the consumer makes it clear it is subject to all the filtering and policies configured there maybe?

ripienaar · 2024-01-25T10:23:06Z

jetstream/consumer_config.go

+		// message, including its sequence numbers and timestamp.
+		Delivered SequenceInfo `json:"delivered"`
+
+		// AckFloor tracks the highest sequence number of a message that has


Indicates the message before the first unacknowledged message - maybe something like that? It is confusing for sure

jetstream/stream_config.go

ripienaar · 2024-01-25T11:15:54Z

jetstream/stream_config.go

+		Deleted []uint64 `json:"deleted"`
+
+		// NumDeleted is the number of messages that have been removed from the
+		// stream.


is a bit more nuance than just have been removed. It's about gaps so if the stream has 1,10,11 deleted is 9, but if the stream has 10,11 deleted is 0

Yes, that's another weird tough thing to explain concisely...

Changed to:

// NumDeleted is the number of messages that have been removed from the // stream. Only deleted messages causing a gap in stream sequence numbers // are counted. Messages deleted at the beginning or end of the stream // are not counted.

ripienaar · 2024-01-25T11:16:57Z

jetstream/stream_config.go

+		// messages on.
+		NumSubjects uint64 `json:"num_subjects"`
+
+		// Subjects is a list of subjects the stream has received messages on.


list of subjects and message counts per subject.

If asked do you do paged requests to get them all behind the scenes?

Yes, since recently I do (it was actually missing in the new API)

jetstream/stream_config.go

jclasley · 2024-01-25T20:15:46Z

Thank you so much for this, it is going to make adoption of NATS easier for teams and reinforces my faith in the team moving forward!

Signed-off-by: Piotr Piotrowski <[email protected]>

piotrpio · 2024-01-26T13:31:01Z

Thanks for the detailed reviews. I believe I address all of the comments (still not sure what to do with the links: #1532 (comment)).

ripienaar

amazing, some minor nits else LGTM

ripienaar · 2024-01-26T14:00:04Z

jetstream/kv_options.go

@@ -0,0 +1,89 @@
+package jetstream


needs copyright blurb

ripienaar · 2024-01-26T14:00:38Z

jetstream/object_options.go

@@ -0,0 +1,26 @@
+package jetstream


Signed-off-by: Piotr Piotrowski <[email protected]>

Jarema

LGTM!

Great effort!

evanofslack · 2024-01-29T18:08:20Z

I just wanted to mention, these doc additions are super helpful, thanks!

piotrpio added 2 commits January 24, 2024 12:13

[IMPROVED] Documentation for stream and consumer structs

3a04a9f

Signed-off-by: Piotr Piotrowski <[email protected]>

[IMPROVED] Documentation in jetstream package

235b7a5

Signed-off-by: Piotr Piotrowski <[email protected]>

piotrpio requested a review from Jarema January 24, 2024 16:32

Jarema requested changes Jan 25, 2024

View reviewed changes

piotrpio and others added 2 commits January 25, 2024 10:06

Update comment on Sealed stream property

8f72d1f

Co-authored-by: Tomasz Pietrek <[email protected]>

Update jetstream/consumer.go

b5ae4f4

Co-authored-by: Tomasz Pietrek <[email protected]>

ripienaar requested changes Jan 25, 2024

View reviewed changes

piotrpio force-pushed the improve-jetstream-docs branch 2 times, most recently from 48e18b2 to d5135d8 Compare January 26, 2024 13:21

Apply review comments

77ba6cc

Signed-off-by: Piotr Piotrowski <[email protected]>

piotrpio force-pushed the improve-jetstream-docs branch from d5135d8 to 77ba6cc Compare January 26, 2024 13:23

piotrpio requested review from ripienaar and Jarema January 26, 2024 13:31

ripienaar approved these changes Jan 26, 2024

View reviewed changes

Added missing copyright

c572815

Signed-off-by: Piotr Piotrowski <[email protected]>

Jarema approved these changes Jan 26, 2024

View reviewed changes

piotrpio merged commit 94eaa2a into main Jan 26, 2024
2 checks passed

piotrpio deleted the improve-jetstream-docs branch January 26, 2024 18:44

Jarema mentioned this pull request Feb 1, 2024

JetStreamManager.AddConsumer ambiguous behaviour with existing consumer #1542

Closed

piotrpio mentioned this pull request Feb 14, 2024

Release v1.33.0 #1556

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve `jetstream` documentation #1532

Improve `jetstream` documentation #1532

piotrpio commented Jan 24, 2024

Jarema left a comment

Jarema Jan 25, 2024

piotrpio Jan 26, 2024

Jarema Jan 25, 2024

piotrpio Jan 25, 2024

piotrpio Jan 26, 2024

ripienaar left a comment

ripienaar Jan 25, 2024

piotrpio Jan 26, 2024

ripienaar Jan 25, 2024

piotrpio Jan 26, 2024

ripienaar Jan 25, 2024

ripienaar Jan 25, 2024

piotrpio Jan 26, 2024

piotrpio Jan 26, 2024

ripienaar Jan 25, 2024

piotrpio Jan 26, 2024

jclasley commented Jan 25, 2024

piotrpio commented Jan 26, 2024

ripienaar left a comment

ripienaar Jan 26, 2024

ripienaar Jan 26, 2024

Jarema left a comment

evanofslack commented Jan 29, 2024

Improve jetstream documentation #1532

Improve jetstream documentation #1532

Conversation

piotrpio commented Jan 24, 2024

Jarema left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ripienaar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jclasley commented Jan 25, 2024

piotrpio commented Jan 26, 2024

ripienaar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jarema left a comment

Choose a reason for hiding this comment

evanofslack commented Jan 29, 2024

Improve `jetstream` documentation #1532

Improve `jetstream` documentation #1532