ShardFollowNodeTask should fetch operation once

[dnhatn]

Today ShardFollowNodeTask might fetch some operations more than once.
This happens because we ask the leading for up to max_batch_count
operations (instead of the left-over size) for the left-over request.
The leading then can freely respond up to the max_batch_count, and at
the same time, if one of the previous requests completed, we might issue
another read request whose range overlaps with the response of the
left-over request.

Closes #32453

[elasticmachine]

Pinging @elastic/es-distributed

[bleskes]

I probably don't understand something I want to clarify. The idea of asking for more then the global checkpoint was that when we do that, we know it's the last request we're going to send out and it's also the only one requesting that range - might as well ask for more (until the request comes back with a new global checkpoint, we won't ask for anything else and when it does come back new requests will take the already fetched operations will be taken into account).

The log line in the bug report said:

[ShardFollowNodeTask] fetch from=1620, to=1679, receive [1620 2024] (*)

but I can't find that pattern in code, so I can't clarify what it means exactly.

What am I missing?

PS I'm not saying we shouldn't make the change you're suggesting but I want to understand it better. Regardless, it should be ok to fetch things twice in rare cases.

[dnhatn]

@bleskes

[ShardFollowNodeTask] fetch from=1620, to=1679, receive [1620 2024] (*)
but I can't find that pattern in code, so I can't clarify what it means exactly.

Sorry for the confusion. I added this log locally for debugging.

The idea of asking for more then the global checkpoint was that when we do that, we know it's the last request we're going to send out and it's also the only one requesting that range - might as well ask for more (until the request comes back with a new global checkpoint, we won't ask for anything else and when it does come back new requests will take the already fetched operations will be taken into account).

If the requesting is the only ongoing request, we should ask for max_batch_count. However, here I am fixing the left-over request, not the peak request (MVG's term).

Suppose the leader_global_checkpoint (fetched by the task) is 2018, last_request_seqno is 2, max_batch_count is 1000. The current code will send concurrently three requests:

• ( from=2, batch_count=1000, max_required=1001 ),
• ( from=1002, batch_count=1000, max_required=2001 ),
• ( from=2002, batch_count=1000, max_required=2018 )

The last_request_seqno is 2018 after issuing these three requests.

If the global checkpoint on the leader has advanced to 2999; when the first request completed, we will another read request (from=2019, batch_count=1000, max_required=2999). This request will receive operations from 2019 to 2999. The problem is that the third request (from=2002, batch_count=1000, max_required=2018) will also receive operations from 2002 to 2999. Here we fetch 2019 to 2999 twice.

PS I'm not saying we shouldn't make the change you're suggesting but I want to understand it better. Regardless, it should be ok to fetch things twice in rare cases.

Yep, I agree we should not enforce this all the time, but we should avoid in obvious cases.

[bleskes]

LGTM. Thanks for clarifying.

[bleskes]

add "add let it optimistically fetch more documents if possible (but not require it)"?

[dnhatn]

Thanks @bleskes for reviewing.