Do not set a default for MaxAckPending

kozlovic commented 2 years ago

Overview

Some clients have set a MaxAckPending (when not explicitly set by the user) to a very high value. Library should leave this non-set and let the server pick its default. However, the library may want to ensure that the NATS subscription's pending limits are covering the MaxAckPending value.

Clients and Tools

[x] CLI @ripienaar
[x] Go @wallyqs
[x] Java @scottf
[x] JavaScript @aricart
[x] .Net @ColinSullivan1 @scottf
[x] C @kozlovic
[x] Python @wallyqs
[x] Ruby @wallyqs
[x] Rust @Jarema

Other Tasks

[x] docs.nats.io updated
[ ] Update ADR to Implemented
[x] 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.

scottf commented 2 years ago

@kozlovic So currently the java and .net clients already do not set a default Max Ack Pending, i.e. don't send it unless the user sets a value. BUT the server currently returns 20,000 instead of leaving it blank, which I rely on to as equivalent to unset by the user. Can the server be changed to not return a value if the consumer was created without one?

kozlovic commented 2 years ago

@scottf No, the server has to set a value, which defaults to 20000 indeed. I am not sure what you mean by: "which I rely on to as equivalent to unset by the user".

scottf commented 2 years ago

So when create a consumer with a default consumer configuration, the server returns this:

{ "durable_name": "durable",
 "deliver_policy": "all",
 "ack_policy": "explicit",
 "ack_wait": 30000000000,
 "max_deliver": -1,
 "replay_policy": "instant",
 "max_waiting": 512,
 "max_ack_pending": 20000 }

I'm just wondering if the server could not return 20000, don't send the field at all for non zero and non negative values that are the default, which would match what is used on create. The same for max_waiting. The client does not send any values to the server to set them but something is returned.

As far as "rely on as equivalent" when subscribing, we check for an exact match of the consumer configuration. I allow the user to have set 200000 for max_ack_pending and to have set max_waiting to 512 to be the equivalent of server default. Maybe this is a bad idea, but it I did it only b/c the server set these defaults in responses.

Either way, the Java and .NET client already implement this issue as requested so I've marked that work as done.

kozlovic commented 2 years ago

the server to set them but something is returned.

Because this is the actual consumer configuration. The server needs to set values that are persisted and return to consumer info APIs (or response to create).

we check for an exact match of the consumer configuration

And I mentioned in the past that the configuration check needs to ignore values that have not been explicitly set by the user. So for instance in Go this would look like:

    if u.MaxAckPending > 0 && u.MaxAckPending != s.MaxAckPending {
        return makeErr("max ack pending", u.MaxAckPending, s.MaxAckPending)
    }

when u is the config from the user, and s from the "server" (the consumer info we get back from the server). So we check only if explicitly set.

caspervonb commented 2 years ago

No change needed in the Rust client, already an optional value Option<i64> which has no default behavior if not set (validation for binding is also skipped when None as that indicated that the user did not care).

nats-io / nats-architecture-and-design