[Enhancement]: Pass mvccTS rather than BeginTS for Iterators

[PwzXxm]

Is there an existing issue for this?

• I have searched the existing issues

What would you like to be added?

Use mvccTS rather than BeginTS in the first search call of the Iterator, affecting seach/query iterators.

Why is this needed?

The second next() becomes slower than the first and the third.
One or two magnitude slower.

Anything else?

No response

[xiaofan-luan]

Is there an existing issue for this?

• I have searched the existing issues

What would you like to be added?

Use mvccTS rather than BeginTS in the first search call of the Iterator, affecting seach/query iterators.

Why is this needed?

The second next() becomes slower than the first and the third. One or two magnitude slower.

Anything else?

No response

what is beginTS? why does that has impact on performance?

[congqixia]

@xiaofan-luan the background of this issue was to use same mvcc timestamp for each sub requests of SearchIterator
current implementation is using BeginTs() (see code below)

milvus/internal/proxy/task_search.go

Lines 744 to 749 in fd72151

	t.result.Results.OutputFields = t.userOutputFields
	t.result.CollectionName = t.request.GetCollectionName()
	if t.isIterator && t.request.GetGuaranteeTimestamp() == 0 {
	// first page for iteration, need to set up sessionTs for iterator
	t.result.SessionTs = t.BeginTs()
	}

BeginTs() is almost current ts (since first search is quick say about 10ms), then second sub search request is like using strong consistency level and take 200ms +

[PwzXxm]

The current implementation is returning the current ts of the first seach/query request and send back to SDK. The sdk captures this and send the following requests with this TS, which results in the second request needs to catch up, could take extra 200ms if unlucky

Ideally, we could use mvcc ts in the first request, so that the following requests does not need to catch up

[PwzXxm]

/assign @MrPresent-Han

[PwzXxm]

For detailed explanation, see #37180

[PwzXxm]

Consistency Level Eventually and Session may lose results on 2.5/master branch. Fixing.

[PwzXxm]

Copy from

For detailed explanation, see #37180

BeginTS is the current timestamp when the task is created.
The GuaranteeTS is the one parsed based on both consistency level and beginTS, in PreExecute of the task on Proxy.
The delegator will wait until GuaranteeTS is met.
In PostExecute of the task on Proxy, the TS of the first iterator request will be returned to the SDK and add it to the subsequent requests.

Hence, if the default consistency level is Eventually or Bounded, the order of TS will be

Guarantee TS < BeginTS

If it returns the BeginTS, the second request will need to catch up and result in extra 200ms max of latency, which results in something like

Call	Latency
first call on Next()	30ms
second call on Next()	210ms
third call on Next()	10ms
fourth call on Next()	11 ms
...	...

where we expect

Call	Latency
first call on Next()	30ms
second call on Next()	10ms
third call on Next()	10ms
fourth call on Next()	11 ms
...	...

[PwzXxm]

Hence, we return the MvccTs (tsafe) from the first request. Will deal with multiple shards in a separate issue/PR.

[xiaofan-luan]

Hence, we return the MvccTs (tsafe) from the first request. Will deal with multiple shards in a separate issue/PR.

Is there a difference between multi shards and single shard or simply some extra work?

[MrPresent-Han]

/unassign

[MrPresent-Han]

/assign PwzXxm