[
https://issues.apache.org/jira/browse/CASSANDRA-9318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15356882#comment-15356882
]
Sergio Bossa commented on CASSANDRA-9318:
-----------------------------------------
Thanks for reviewing [~Stefania]! See my comments below...
bq. If I understood correctly, what we are trying to achieve with this new
approach, is reducing the number of dropped mutations rather than preventing
OOM but, in a way that doesn't cause OOM by invoking a rate-limiter-acquire in
a Netty shared worker pool thread, which would slow down our ability to handle
new client requests, as well as throw OEs once in a while when a low water-mark
is reached. Is this a fair description?
Correct. I'd just highlight the fact that reducing dropped mutations and
replica instability is to me as important as avoiding OOMEs on the coordinator;
[~jshook] has some very good points about that on his comment above.
bq. In the CCM test performed with two nodes, what was the difference in write
timeout?
For that specific test I've got no client timeouts at all, as I wrote at ONE.
bq. In terms of testing, here is what I would suggest
Agreed with all your points. I'll see what I can do, but any help/pointers will
be very appreciated.
bq. As for the code review, I've addressed some nits, mostly comments and
trailing spaces, directly here.
Looks good! Please send me a PR and I'll incorporate those in my branch :)
bq. Should we perhaps group all settings under something like
back_pressure_options?
I find the current layout effective and simple enough, but I'll not object if
you want to push those under a common "container" option.
bq. back_pressure_timeout_override: should we call it for what it is, the
back_pressure_window_size and recommend that they increase the write timeout in
the comments, or is an override of the write timeout necessary for correctness?
I don't like much that name either, as it doesn't convey very well the (double)
meaning; making the back-pressure window the same as the write timeout is not
_strictly_ necessary, but it makes the algorithm behave better in terms of
reducing dropped mutations as it gives replica more time to process its backlog
after the rate is reduced. Let me think about that a bit more, but I'd like to
avoid requiring the user to increase the write timeout manually, as again, it
reduces the effectiveness of the algorithm.
bq. Should we use a dedicated package?
I'd keep it plain as it is now.
bq. Depending on the precision, in trunk there is an approximate time service
with a precision of 10 ms, which is faster than calling
System.currentTimeMillis()
Sure I can switch to that on trunk, if you think it's worth performance-wise (I
can write a JMH test if there isn't one already).
bq. TESTRATELIMITER.JAVA - Is this still used?
It is not used in any unit tests code, but it is used in my manual byteman
tests, and unfortunately I need it on the C* classpath; is that a problem to
keep it? (I can add proper javadocs to make that clear)
bq. TESTTIMESOURCE.JAVA - I think we need to move it somewhere with the test
sources.
Leftover there from a previous test implementation; I'll move it.
> Bound the number of in-flight requests at the coordinator
> ---------------------------------------------------------
>
> Key: CASSANDRA-9318
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9318
> Project: Cassandra
> Issue Type: Improvement
> Components: Local Write-Read Paths, Streaming and Messaging
> Reporter: Ariel Weisberg
> Assignee: Sergio Bossa
> Attachments: backpressure.png, limit.btm, no_backpressure.png
>
>
> It's possible to somewhat bound the amount of load accepted into the cluster
> by bounding the number of in-flight requests and request bytes.
> An implementation might do something like track the number of outstanding
> bytes and requests and if it reaches a high watermark disable read on client
> connections until it goes back below some low watermark.
> Need to make sure that disabling read on the client connection won't
> introduce other issues.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
