[
https://issues.apache.org/jira/browse/CASSSIDECAR-510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18126751#comment-18126751
]
n.v.harikrishna commented on CASSSIDECAR-510:
---------------------------------------------
Thanks for the analysis. You're right that the traffic shaper turns off
zero-copy {{sendfile}}. Here are some initial thoughts:
- When sidecars send a lot of data, a bandwidth limit can stop them from
saturating the network. The traffic shaper does this today, and I think it's
useful in production.
- Vert.x also skips zero-copy when SSL is on, even without the shaper. Many
users may want to keep HTTPS, so removing the shaper might not speed things up
for them.
- The shaper applies to every connection on the server, which includes live
migration, SSTable streaming and uploads (used by analytics), CDC segments and
the other APIs. It would be good to keep that global coverage rather than
covering only live migration or file stream cases.
- I would be fine with turning off the shaper when the user explicitly
configures it that way. I think it would be better as an _opt-in_ option rather
than removing the shaper completely. Users who want the best plaintext speed
could choose that, and everyone else would keep the protection.
- As far as I can see, {{StreamRequestRateLimiter}} in {{FileStreamer}} limits
the number of file requests per second, not the bytes sent. A global byte limit
seems like a better fit than adding rate limiters in a few handlers, since new
endpoints could easily be missed that way.
I'll spend some more time on this and get back with more inputs.
> Replace Netty GlobalTrafficShapingHandler with userspace byte-counting rate
> limiter so file downloads can use kernel sendfile(2)
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: CASSSIDECAR-510
> URL: https://issues.apache.org/jira/browse/CASSSIDECAR-510
> Project: Sidecar for Apache Cassandra
> Issue Type: Improvement
> Components: Performance
> Reporter: Sampreeth Reddy B
> Assignee: Sampreeth Reddy B
> Priority: Major
>
> h3. Description
> h4. Problem:
> [{{HttpServerOptionsProvider.apply(SidecarConfiguration)}}|https://github.com/apache/cassandra-sidecar/blob/trunk/server/src/main/java/org/apache/cassandra/sidecar/server/HttpServerOptionsProvider.java]
> unconditionally calls
> {{options.setTrafficShapingOptions(buildTrafficShapingOptions(…))}} on every
> sidecar startup:
> {quote}options.setTrafficShapingOptions(buildTrafficShapingOptions(serviceConf.trafficShapingConfiguration()));{quote}
> Only install the shaper when a cap is configured does not exist. The
> {{buildTrafficShapingOptions(…)}} helper is invoked with the live config
> values even when the operator sets
> {quote}{{{}inboundGlobalBandwidthBytesPerSecond=0{}}},
> {{{}outboundGlobalBandwidthBytesPerSecond=0{}}}, and
> {{{}peakOutboundGlobalBandwidthBytesPerSecond=0{}}},
> {quote}
> the returned {{TrafficShapingOptions}} object is non-null, so Vert.x's
> {{HttpServerImpl}} adds a Netty {{GlobalTrafficShapingHandler}} to every
> connection's pipeline anyway.
> Installing _any_ {{io.netty.handler.traffic.AbstractTrafficShapingHandler}}
> on a Netty pipeline has one critical side effect that is not documented in
> Vert.x's public API:
> {{io.vertx.core.net.impl.ConnectionBase#supportsFileRegion()}} returns
> {{false}} whenever the pipeline contains an
> {{{}AbstractTrafficShapingHandler{}}}. That return value is checked inside
> Vert.x's {{AsyncFileImpl#doSendFile(…)}} and inside
> {{Http1xServerResponse#sendFile(…)}} when it returns {{{}false{}}}, Vert.x
> silently replaces the {{DefaultFileRegion}} zero-copy path with a userspace
> {{{}read(){}}}-into-{{{}ByteBuf{}}} + {{write()}} loop.
> Net effect: every live-migration download and every SSTable stream on every
> release of the sidecar is served through userspace copy, not kernel
> {{sendfile(2)}}, regardless of whether the operator asked for bandwidth
> throttling.
> h4. Proposed Change
> Remove the Netty {{GlobalTrafficShapingHandler}} entirely and move byte-level
> rate enforcement into the existing per-file streaming loop as a new
> {{SidecarRateLimiter}} injected binding whose unit is bytes per second (the
> existing class supports arbitrary permit units it's just a wrapper around
> Guava's {{{}SmoothBurstyRateLimiter{}}}).
> High-level design:
> # Delete the pipeline shaper. In {{{}HttpServerOptionsProvider.apply(…){}}},
> drop the {{options.setTrafficShapingOptions(…)}} call. The
> {{TrafficShapingConfiguration}} interface is kept for one release as a
> deprecated no-op (log a WARN at startup if any non-zero value is set,
> pointing operators at the new config section).
> # New byte-counting limiter binding.
> #
> {{{}{}}}Chunk the sendfile call. Replace the single
> {{response.sendFile(filename, fileLength, range)}} call inside
> [{{FileStreamer.acquireAndSend(…)}}|https://github.com/apache/cassandra-sidecar/blob/trunk/server/src/main/java/org/apache/cassandra/sidecar/utils/FileStreamer.java]
> with a chunked loop (default 256 KiB chunk, configurable):
> # Keep {{SidecarRateLimiter}} ("StreamRequestRateLimiter") for admission
> control. It still has a legitimate role rejecting new file-stream requests
> when the server is saturated at the connection-count level, which is
> semantically different from slow this stream down. Rename the binding to
> {{"StreamAdmissionRateLimiter"}} to reduce confusion.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
