Botond Dénes
Botond Dénes
> Beyond the general discussion about the concurrency of cpu-bound reads which I'm trying to hold in the issue, not in this PR, I have a question about this PR:...
> Further to that, the patch helps only if only a small fraction of reads hit those large collections. If a larger fraction hits, increasing concurrency makes things worse, since...
> > Yes. Large collections are just plain bad. An alternative solution would be if a read could detect reading a large collection of a certain size and would disqualify...
New in v2: * Fix message in db::config. * reader_concurrency_semaphore: s/all_need_cpu_permits_are_awaiting/cpu_concurrency_limit_reached/ (logic is now negated) Still looking into how to write a test for this. Our current infra in perf/...
> > New in v2: > > > > * Fix message in db::config. > > * reader_concurrency_semaphore: s/all_need_cpu_permits_are_awaiting/cpu_concurrency_limit_reached/ (logic is now negated) > > > > Still looking into...
> @denesb - what's the next step here? I'm trying to come up with the requested reproducer test and I'm failing. I have written the test, but no matter how...
v3: * Rebased and resolved conflicts (there are less updated constructor callers now, there is a simplified constructor which is backwards compatible). * Added unit test for the new functionality....
I don't claim this PR to be an improvement, the reason I made this a tunable instead of just bumping it to some other value, is exactly because I'm not...
> Why the second reader in (1) is put in the _waiter_list and not in the _ready_list if there are enough permits to do so, @michoecho ? The size of...
@bhalevy the fix is this: ```diff diff --git a/reader_concurrency_semaphore.cc b/reader_concurrency_semaphore.cc index afd6c88d0a..70e7cea2f6 100644 --- a/reader_concurrency_semaphore.cc +++ b/reader_concurrency_semaphore.cc @@ -1354,7 +1354,7 @@ reader_concurrency_semaphore::can_admit_read(const reader_permit::impl& permit) return {can_admit::yes, reason::all_ok}; } - if...