[
https://issues.apache.org/jira/browse/CASSANDRA-9318?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15386978#comment-15386978
]
Stefania edited comment on CASSANDRA-9318 at 7/21/16 1:51 AM:
--------------------------------------------------------------
bq. I think if we add too many responsibilities to the strategy it really
starts to look less and less as an actual strategy; we could rather add a
"BackPressureManager", but to be honest, the current responsibility assignment
feels natural to me, and back-pressure is naturally a "messaging layer"
concern, so I'd keep everything as it is unless you can show any actual
advantages in changing the design.
I don't think we want a "BackPressureManager". We just need to decide if we are
happy having a back pressure state for each OutboundTcpConnectionPool, even
though back pressure may be disabled or some strategies may use different
metrics, e.g. total in-flight requests or memory, or whether the states should
be hidden inside the strategy.
bq. This was intentional, so that outgoing requests could have been processed
while pausing; but it had the drawback of making the rate limiting less
"predictable" (as dependant on which thread was going to be called next), so I
ended up changing it.
Now the back pressure is applied before hinting, or inserting local mutations,
or sending to remote data centers, but _after_ sending to local replicas. I
think we may need to rework SP.sendToHintedEndpoints a little bit, I think we
want to fire the insert local, then block, then send all messages.
There is one more potential issue in the case of *non local replicas*. We send
the mutation only to one remote replica, which then forwards it to other
replicas in that DC. So remote replicas may have the outgoing rate set to zero,
and therefore the limiter rate set to positive infinity, which means we won't
throttle at all with FAST flow selected. In fact, we have no way of reliably
knowing the status of remote replicas, or replicas to which we haven't sent any
mutations recently, and we should perhaps exclude them.
bq. It's not a matter of making the sort stable, but a matter of making the
comparator fully consistent with equals, which should be already documented by
the treeset/comparator javadoc.
It's in the comparator javadoc but not the TreeSet constructor doc, that's why
I missed it yesterday. Thanks.
bq. Agreed. If we all (/cc Aleksey Yeschenko Sylvain Lebresne Jonathan Ellis)
agree on the core concepts of the current implementation, I will rebase to
trunk, remove the trailing spaces, and then I would move into testing it, and
in the meantime think/work on adding metrics and making any further adjustments.
I think we can start testing once we have sorted the second discussion point
above, as for the API issues, we'll eventually need to reach consensus but we
don't need to block the tests for it.
One more tiny nit:
* The documentation of {{RateBasedBackPressureState}} still mentions the
overloaded flag.
was (Author: stefania):
bq. I think if we add too many responsibilities to the strategy it really
starts to look less and less as an actual strategy; we could rather add a
"BackPressureManager", but to be honest, the current responsibility assignment
feels natural to me, and back-pressure is naturally a "messaging layer"
concern, so I'd keep everything as it is unless you can show any actual
advantages in changing the design.
I don't think we want a "BackPressureManager". We just need to decide if we are
happy having a back pressure state for each OutboundTcpConnectionPool, even
though back pressure may be disabled or some strategies may use different
metrics, e.g. total in-flight requests or memory, or whether the states should
be hidden inside the strategy.
bq. This was intentional, so that outgoing requests could have been processed
while pausing; but it had the drawback of making the rate limiting less
"predictable" (as dependant on which thread was going to be called next), so I
ended up changing it.
Now the back pressure is applied before hinting, or inserting local mutations,
or sending to remote data centers, but _after_ sending to local replicas. I
think we may need to rework SP.sendToHintedEndpoints a little bit, I think we
want to fire the insert local, then block, then send all messages.
There is one more potential issue in the case of *non local replicas*. We send
the mutation only to one remote replica, which then forwards it to other
replicas in that DC. So remote replicas may have the outgoing rate set to zero,
and therefore the limiter rate set to positive infinity, which means we won't
throttle at all with FAST flow selected. In fact, we have no way of reliably
knowing the status of remote replicas, or replicas to which we haven't sent any
mutations recently, and we should perhaps exclude them.
bq. It's not a matter of making the sort stable, but a matter of making the
comparator fully consistent with equals, which should be already documented by
the treeset/comparator javadoc.
It's in the comparator javadoc but not the TreeSet constructor odc, that's why
I missed it yesterday. Thanks.
bq. Agreed. If we all (/cc Aleksey Yeschenko Sylvain Lebresne Jonathan Ellis)
agree on the core concepts of the current implementation, I will rebase to
trunk, remove the trailing spaces, and then I would move into testing it, and
in the meantime think/work on adding metrics and making any further adjustments.
I think we can start testing once we have sorted the second discussion point
above, as for the API issues, we'll eventually need to reach consensus but we
don't need to block the tests for it.
One more tiny nit:
* The documentation of {{RateBasedBackPressureState}} still mentions the
overloaded flag.
> 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: 9318-3.0-nits-trailing-spaces.patch, 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)
