I've been meaning to reply to this - it's time! I suspect that #1 is
just historical, and happened because the limiter's initial focus was on
keeping LSM activity (in particular big merges) from overrunning the
other activities in the system. It seems like this is the right move,
IMO. I wouldn't worry about back-compat for this - it seems fine since
by default this is not enabled (so I don't think we have use cases that
are depending on it not changing).
On 5/3/23 11:43 AM, Ian Maxon wrote:
Hi everyone,
I've been working on a patch that adds read rate limiting to AsterixDB so
that multiple NCs can share the same disk in a more cooperative fashion.
Clearly since we already have write rate limiting, I looked at how that is
done as a first step. It seemed easy enough to perform the read rate
limiting precisely the same way as the write rate limiting, just that it
needs to be for all IO, not just LSM IO. So, the best place to put the
limiter seemed to be in the IOManager. Given that, I had a few questions
that came to mind:
1. Why is the write-rate limiting in the LSM page write callback instead
of further down, like in the IOManager?
2. Is there any downside to moving it to the IOManager?
3. Our rate limiter at the moment wraps Guava's RateLimiter. We specify
in the constructor a maxBurstSeconds argument, but where we give this to
Guava the meaning is not clear to me. From my reading of the docs and the
source comments, it seems like what we pass as a burst time is actually a
warmup time, and that the burst time is fixed to be 1 second within Guava's
RateLimiter as long as you use public constructors. Is this the case or am
I misreading something?
The main concern I have is with backwards compatibility, because I would
like to reuse the current config parameter for write rate limiting to just
mean all writes going through the IOManager, not just LSM writes. If they
both achieve the same purpose then it would be fine, but if they are
different then of course something else needs to be done.
I have a WIP patch up to illustrate, it has a couple warts but hopefully
the idea is clear:https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17497
Any thoughts are much appreciated.
Thanks,
- Ian
