fix: change sequential approach to parallel for Iterator first page

[l2ysho]

Batch concurrent record fetching in KVS values() and entries() Promise path

Summary

The values() and entries() methods on KeyValueStoreClient previously fetched records sequentially in their eager Promise path (used when callers await the result directly). For stores with many keys, this meant each getRecord() call blocked before the next could start. This change replaces the sequential loop with Promise.allSettled gated by p-limit (concurrency of 25), fetching records in parallel while preserving order and gracefully skipping missing keys.

closes #3395

[l2ysho]

Quick PR to address our discussion in issue. I introduced GET_RECORD_CONCURRENCY constant, can someone suggest a safe value for concurency for getRecords? (25 now) @barjin @B4nan

[B4nan]

can someone suggest a safe value for concurency for getRecords?

25 sounds all right to me. But if you wanna be safe, you can ask someone from other teams on slack I guess.

[B4nan]

Now I see you used this only in the memory storage, in that case 25 sounds all right and it makes no sense to raise this on slack, since we are not firing any API requests at all, right?

[l2ysho]

Now I see you used this only in the memory storage, in that case 25 sounds all right and it makes no sense to raise this on slack, since we are not firing any API requests at all, right?

true 👍

[barjin]

Looks pretty straightforward, thank you @l2ysho !

One note: this doesn't (fully) solve #3395, as the iterator still waits for the full page to be loaded before yielding the first item. This is what I'm describing in #3395 (comment).

Nonetheless, this is a step forward. 👍

[barjin]

nit: record is { key:string, value: any } already, maybe you can just pass record here?

crawlee/packages/types/src/storages.ts

Lines 124 to 127 in 136d368

	export interface KeyValueStoreRecord {
	key: string;
	value: any;
	contentType?: string;

[l2ysho]

Looks pretty straightforward, thank you @l2ysho !

One note: this doesn't (fully) solve #3395, as the iterator still waits for the full page to be loaded before yielding the first item. This is what I'm describing in #3395 (comment).

Nonetheless, this is a step forward. 👍

yop, I should get rid of IEFF and figure out some lazy loading for the first page 🤔

const result = {
    then(resolve: any, reject: any) {
        return getFirstPageEntries().then(resolve, reject);
    },
    [Symbol.asyncIterator]: asyncGenerator,
};

But it started to be complex to read even without this. I will merge this as is and we can iterate it later.

[barjin]

we can iterate later

Agreed, please make/reopen an issue for that (the original one got closed by this PR).