[JS] experimental API pull with max_bytes returns only first message

[aricart]

Defect

Given a consumer that has 100 pending messages:

{
  type: "io.nats.jetstream.api.v1.consumer_info_response",
  stream_name: "K54T0380942R0POQHUKQ2I",
  name: "K54T0380942R0POQHUKQ2I",
  created: "2023-01-25T20:54:11.822784Z",
  config: {
    durable_name: "K54T0380942R0POQHUKQ2I",
    name: "K54T0380942R0POQHUKQ2I",
    deliver_policy: "all",
    ack_policy: "none",
    max_deliver: -1,
    replay_policy: "instant",
    max_waiting: 512,
    num_replicas: 0
  },
  delivered: { consumer_seq: 0, stream_seq: 0 },
  ack_floor: { consumer_seq: 0, stream_seq: 0 },
  num_ack_pending: 0,
  num_redelivered: 0,
  num_waiting: 0,
  num_pending: 100
}

Doing a pull request like this:

PUB $JS.API.CONSUMER.MSG.NEXT.K54T0380942R0POQHUKQ2I.K54T0380942R0POQHUKQ2I _INBOX.K54T0380942R0POQHUL57X 78␍␊{"batch":0,"max_bytes":1024,"idle_heartbeat":5000000000,"expires":10000000000}␍␊

Will yield a single message from the server. The server will not finalize the request, and no heartbeats are sent. The client will fail because of missed heartbeats. If batch is set to something, ie 1000 - the request will succeed, heartbeat properly and send the finalization message telling how many bytes/messages are pending before the expiration.

This happens all the way from 2.9.4 including current dev (2.11.x)

Steps or code to reproduce the issue:

Sample program to exercise this:
https://github.com/aricart/js-simplification-examples/blob/main/max_bytes_issue.ts

nats-server -js -sd /tmp/jsdata &
deno run -A https://raw.githubusercontent.com/aricart/js-simplification-examples/main/max_bytes_issue.ts
{
  type: "io.nats.jetstream.api.v1.consumer_info_response",
  stream_name: "K54T0380942R0POQHUKQ2I",
  name: "K54T0380942R0POQHUKQ2I",
  created: "2023-01-25T20:54:11.822784Z",
  config: {
    durable_name: "K54T0380942R0POQHUKQ2I",
    name: "K54T0380942R0POQHUKQ2I",
    deliver_policy: "all",
    ack_policy: "none",
    max_deliver: -1,
    replay_policy: "instant",
    max_waiting: 512,
    num_replicas: 0
  },
  delivered: { consumer_seq: 0, stream_seq: 0 },
  ack_floor: { consumer_seq: 0, stream_seq: 0 },
  num_ack_pending: 0,
  num_redelivered: 0,
  num_waiting: 0,
  num_pending: 100
}
< SUB _INBOX.K54T0380942R0POQHUL57X 2␍␊PUB $JS.API.CONSUMER.MSG.NEXT.K54T0380942R0POQHUKQ2I.K54T0380942R0POQHUKQ2I _INBOX.K54T0380942R0POQHUL57X 78␍␊{"batch":0,"max_bytes":1024,"idle_heartbeat":5000000000,"expires":10000000000}␍␊
> MSG K54T0380942R0POQHUKQ2I.x 2 $JS.ACK.K54T0380942R0POQHUKQ2I.K54T0380942R0POQHUKQ2I.1.1.1.1674680051829863000.99 10␍␊␍␊
1
> PING␍␊
< PONG␍␊

Expected result:

The 100 messages should have been delivered and the protocol messages signaling a heartbeat and completion of the request should have been returned to the client.

[derekcollison]

@Jarema possible to take a look?

[Jarema]

yes. Will take look.

[aricart]

One consideration before we go in a loop on this is that while the idea of having a request that only has max_bytes sounds right, is possibly wrong. Let me explain.

• Processing a message takes N amount of time
• max wait for ack is set to some value
• maximum number of messages that can be processed by a client are clamped by those to values when acks are required

If that is the case, requests may not require the number of messages but they may specify them anyway. This means that the API should allow specifying batch and max_bytes because it will be required anyway. If that is the case the client should default the number which can be documented, and user can then override if their processing requirements require a lower bound on this setting to meet processing within the ack window.

[aricart]

So - on further elucidating, the issue is really that a batch of 0, - the default is set to be 1 message on the server. This is fine, but the issue is that the server didn't sent a sentinel message because it met its default 1 message. When what needs to happen in this case, the sentinel is sent, with the pending bytes that were not satisfied. This would solve the issue. - IE when max_bytes is specified, even if batch is provided, must report the sentinel, the only time the sentinel is not required is when the number of messages and the bytes were satisfied exactly.

[Jarema]

Two thiggs I'm doing:

1. If max_bytes is specified, add Error message that informs client how many bytes were pending. Then we have the same behaviour for max_bytes and batch not fulfilled. That would be useful to clients no matter what. And would not affect traffic if only batch is set.
2. Disable setting batch to 1 by default if max_bytes is passed (not sure if we set max_bytes to 1 if it's json request. checkng) or fixing the bug why 1 is returned.

WIthout nr 1, clients cannot rely on max_bytes.

[derekcollison]

Do we want/need these for a 2.9.12 release?

[wallyqs]

I think we should revise this within the scope of v2.10 and not as the patch release from next week.

[Jarema]

Yes, that is definitely not for the patch.
It can break some user simplification algorithms.

#3822

[wallyqs]

Fixed via #3822