feat(rdb_load): add support for loading huge sets

[andydunstall]

Adds support for loading huge sets (#3760)

[romange]

If i remember correctly, it was more efficient to allocate smaller arrays than a single huge array.
And I agree with you - lets start with a single "static" rule of upto kMaxBlobLen for streaming.

[andydunstall]

ok sure

[romange]

nit: move(item)

[andydunstall]

thanks updated

[romange]

looks good :)

[romange]

you can use rdb_test.cc to add the unit test

[andydunstall]

Sure I've added a unit test

Testing a 5GB set manually with 5m entries (debug populate 1 test 1000 rand type set elements 5000000)

RSS is still 10GB (5GB used), as it still only does a single flush (since kMaxBufSize x kMaxBlobLen > 8m)

It's also now ~20% slower which seems to be due to not reserving the full set up front anymore. Though can include the full set size in Item to reserve on first load if you think it's worth doing

Repeating the test where we flush if pending_read.remaining > 0, RSS < 7GB (5GB used) and it's 20% faster. Not sure if you think thats worth adding or if theres another flushing approach we should use?

[romange]

I think it's worth reserving capacity if it's easy to add. Also we can reduce now kMaxBlobLen to smaller number since we know large sets will just occupy all the items in ItemsBuf.
using pending_read.remaining as a flush signal is also possible and could be a good short term heuristic.

[andydunstall]

Ok sure, I've reduced kMaxBlobLen to 4096 meaning the 5m set is split into ~10 flushes (128x4096 ~= 500k)

And reserved the full set up front

With that loading the 5GB set takes 2.8s (compared to 3.8s before) with RSS ~7GiB (~10GiB before)

[romange]

why did you add this code?

[andydunstall]

A bit of a guess given it already does ts->TryStash below in the non-append case - should i remove?

(Or if you mean why does it continue early in the append state, it's because we don't want to check expiry, add to db_slice or update flags in the append case right?)

[romange]

Lets remove this code.

1. Data tiering does not work with to non-strings.
2. We do not want to stash values that are not yet finalized.

[andydunstall]

sure removed

[romange]

lgtm, plus minor comments

[romange]

Good work @andydunstall . If you want you can follow up with hash-map + zset PR as well.

[andydunstall]

If you want you can follow up with hash-map + zset PR as well

Thanks will do