Pull config changes in ConsumerConfig and JSApiConsumerGetNextRequest

[scottf]

Overview

Support for MaxBytes for pull requests including changes to Consumer Configuration and JSApiConsumerGetNextRequest

See Server PR 3126

The Consumer Configuration now includes max_expires and max_bytes

MaxRequestExpires  time.Duration `json:"max_expires,omitempty"`
MaxRequestMaxBytes int `json:"max_bytes,omitempty"`

The JSApiConsumerGetNextRequest now includes max_bytes and idle_heartbeat

MaxBytes  int           `json:"max_bytes,omitempty"`
Heartbeat time.Duration `json:"idle_heartbeat,omitempty"`

The max_bytes limit makes the server complete a batch request if the total number of bytes sent to a pull request reaches this value. It works in combination with batch count, which means that the pull request will be complete based on whichever completes first (count or size).

The idle_hearbeat causes the server to send messages with "100 Idle Heartbeat" status (note that the value cannot be 50% more than the consumer config heartbeat value, but the check is done by the server, library does not have to check for that).

Note max_bytes is currently EXPERIMENTAL while behavior is ironed out.

Clients and Tools

• [x] Schemas @ripienaar
• [x] CLI @ripienaar
• [x] Terraform @ripienaar
• [x] Github Actions @ripienaar
• [x] Go @piotrpio
• [x] Java @scottf
• [ ] JavaScript @aricart. https://github.com/nats-io/nats.deno/pull/318
• [x] .Net @scottf
• [x] C @kozlovic
• [ ] Python @wallyqs
• [ ] Ruby @wallyqs
• [x] Rust @Jarema

Other Tasks

• [ ] docs.nats.io updated
• [ ] Update ADR to Implemented
• [ ] Update client features spreadsheet

Client authors please update with your progress. If you open issues in your own repositories as a result of this request, please link them to this one by pasting the issue URL in a comment or main issue description.

[kozlovic]

Note that we may not rush that to be added in client libraries until the JS simplification work is done and some discussion occurs between client and server team. For instance, currently the server does not send an indication that the pull request is done because of the size, so the library would need to do accounting of message size + headers when deciding if it needs to wait for more messages or not. Also, in the case where the pull request max_bytes is smaller than the first message that the server would need to deliver, the server sends a 408 with a description "Message Size Exceeds MaxBytes", which would force libraries to behave differently with this 408 than others..

[scottf]

I've marked my additions to pull requests as EXPERIMENTAL so we can change them without a break if we need to.

[aricart]

There are other issues - if the stream has a message that is larger than the max_bytes the consumer will stall, because the server will send a 408, in this case the 408 is final, as there's no way for the message in question to be processed by the client.

https://github.com/nats-io/nats-server/issues/3165

[derekcollison]

Which max_bytes are you explicitly referring to here?

[scottf]

The new max_bytes on a pull (JSApiConsumerGetNextRequest) Let's say you set max_bytes to 1k, but have a message that is 2k. What will happen?

[derekcollison]

You get this..

nats.Header{"Status": []string{"408"}, "Description": []string{"Message Size Exceeds MaxBytes"}}

[scottf]

It would be nice if we didn't have to rely on text for status messages. We face the same issue with flow control and heartbeats, we have to recognize the status code (100) and the text.

For this though, at a minimum I wonder if there is a better code than 408. Maybe one of these?

• 412 Precondition Failed
• 413 Payload Too Large
• 416 Range Not Satisfiable

At a maximum, is there any way we can just ditch the http based codes here and use an api [like] error code?

[derekcollison]

I think that is an excellent idea and happy to make that change. We also of course have a precedence of adding in specific error codes to compliment the status codes, which by design are mimicking HTTP.

[derekcollison]

I don't think we can ditch status alignment with HTTP at this point.

[aricart]

the one thing that is not great about the current strat, is that these errors are reported simply as status errors, there's no attribution, the only thing making them jetstream is that there's no payload. Since they are a JetStream response, it would be great if they were js API errors. - or headers indicated that these are jetstream errors.

[derekcollison]

We could possible add more headers, but not sure if that would buy us much. Sometimes it feels like that just means you need to check multiple headers versus one.

How are you thinking about that would work out in practice and for applications?

[bruth]

Implemented in Go https://github.com/nats-io/nats.go/pull/1043