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

simplified doc #506

Merged
merged 5 commits into from
May 10, 2023
Merged

simplified doc #506

merged 5 commits into from
May 10, 2023

Conversation

aricart
Copy link
Member

@aricart aricart commented May 8, 2023

No description provided.

@aricart aricart temporarily deployed to CI May 10, 2023 21:18 — with GitHub Actions Inactive
Copy link
Member

@bruth bruth left a comment

Choose a reason for hiding this comment

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

LGTM.

jetstream.md Outdated
Comment on lines 12 to 13
specific time. The configuration also specifies if the server should require for
a message to be acknowledged, and how long to wait for acknowledgements. The
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
specific time. The configuration also specifies if the server should require for
a message to be acknowledged, and how long to wait for acknowledgements. The
specific time. The configuration also specifies if the server should require messages
to be acknowledged and how long to wait for acknowledgements. The

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@@ -47,7 +47,7 @@ const stream = "mystream";
const subj = `mystream.*`;
await jsm.streams.add({ name: stream, subjects: [subj] });

// publish a reg nats message directly to the stream
// jetstream can capture nats core messages
Copy link
Member

Choose a reason for hiding this comment

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

I suggest this comment be moved to when the stream is defined. The wording I typically use is "one or more subjects can be bound to a stream". I know "binding" may sound exclusive which we know is not technically true since i can still spin up a sub on the same subject. (Thinking out loud) I also describe a stream as a "responder" to messages on a specific subject whose job is to store them.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

The JetStream client functionality presents a JetStream view on a NATS client.
While you can implement your own API to interact with streams, the JetStream
APIs make this convenient.
The JetStream client presents an API for working with messages stored on a
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The JetStream client presents an API for working with messages stored on a
The JetStream client presents an API for working with messages stored in a

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

const jsm = await nc.jetstreamManager();
await jsm.streams.add({ name: "a", subjects: ["a.*"] });

// create a jetstream client:
const js = nc.jetstream();

// to publish messages to a stream:
// to publish messages to a stream
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// to publish messages to a stream
// publish a message received by a stream

let pa = await js.publish("a.b");
// the jetstream returns an acknowledgement with the
// jetstream returns an acknowledgement with the
Copy link
Member

Choose a reason for hiding this comment

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

One subtle improvement I am attempting with communication is to not refer to jetstream and simply refer to streams and consumers (KV, etc.) as part of NATS. JS is a subsystem, but people should not have to care. Obviously a larger team-wide point to make, but just noting here 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Comment on lines +133 to +135
// for the same ID for a configured amount of time (specified
// by the stream configuration), and reject messages that
// sport the same ID:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// for the same ID for a configured amount of time (specified
// by the stream configuration), and reject messages that
// sport the same ID:
// for the same ID for a configured amount of time (within a
// configurable time window), and reject messages that have
// the same ID:

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

while (true) {
const messages = await c.consume({ max_messages: 1 });

// watch the to see if the consume operation misses heartbeats
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// watch the to see if the consume operation misses heartbeats
// watch to see if the consume operation misses heartbeats

const n = s.data as number;
console.log(`${n} heartbeats missed`);
if (n === 2) {
// by calling `stop()` the message processing loop ends
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// by calling `stop()` the message processing loop ends
// by calling `stop()` the message processing loop ends.

(async () => {
for await (const s of await messages.status()) {
if (s.type === ConsumerEvents.HeartbeatsMissed) {
// you can decide how many heartbeats you are willing to miss
Copy link
Member

Choose a reason for hiding this comment

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

What do the heartbeats semantically correspond to? Availability to the consumer to respond? Not just availability of messages?

Copy link
Member Author

Choose a reason for hiding this comment

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

The server should be sending heartbeats whenever messages are not being sent. This allows the client to know that the server request is alive.

const js = nc.jetstream();
const c = await js.consumers.get(stream, consumer);

// this example uses a consume that is monitors a rate limiter
Copy link
Member

Choose a reason for hiding this comment

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

(?)

Suggested change
// this example uses a consume that is monitors a rate limiter
// this example uses consume that monitors a rate limiter

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to // this example controls parallel processing of the messages // by only allowing 5 concurrent messages to be processed // and then only allowing additional processing as others complete

@aricart aricart merged commit 274dc50 into dev May 10, 2023
1 check passed
@bruth bruth deleted the examples branch May 11, 2023 00:51
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.

None yet

2 participants