Yield every 1000 iterations when fetching sharding keys and funcs#500
Yield every 1000 iterations when fetching sharding keys and funcs#500ita-sammann wants to merge 1 commit into
Conversation
vakhov
left a comment
vakhov
left a comment
There was a problem hiding this comment.
Thanks for the PR! I left a few comments below.
adding explicit yields changes the consistency semantics of this function: concurrent DDL updates may now interleave with the scan, so metadata_map can contain entries from different schema states, including a sharding key and function fetched at different moments. is this eventual consistency guaranteed to be handled by the hash/reload mechanism? could we add a regression test for the cold-cache case to verify that another fiber gets scheduled during a large metadata scan?
| space_name) | ||
| end | ||
|
|
||
| if i % 1000 == 0 then |
There was a problem hiding this comment.
could we extract 1000 into a named constant to avoid duplicating the magic number?
There was a problem hiding this comment.
Good idea, thanks! Extracted
| end | ||
|
|
||
| if i % 1000 == 0 then | ||
| fiber.yield() |
There was a problem hiding this comment.
on a cold cache, get_sharding_(func|key)_hash() calls init_cache(), which scans the whole DDL space without yielding. So the first fetch may still block. should we add yielding inside init_cache() instead?
There was a problem hiding this comment.
Added yield there, thanks for noticing!
ita-sammann
force-pushed
the
fetch-sharding-keys-with-yield
branch
from
86a61f7 to
4fe10ac
Compare
@vakhov, actually I don't like the idea of this yields at all. If there is a sane amount of sharding keys there are no problems. |
2 participants
Yield every 1000 iterations when fetching sharding keys and functions so the server stays responsive when there are millions of records.