uber / ringpop-go

Scalable, fault-tolerant application-layer sharding for Go applications
http://www.uber.com
MIT License
835 stars 83 forks source link

Stop using tchannel.WriteArgs in requestSender.MakeCall #217

Closed jcorbin closed 4 years ago

jcorbin commented 6 years ago

Oh I see... yeah what I'm doing here initially is quite flawed, will sort it out and update.

jcorbin commented 6 years ago

Okay, so I'm pretty sure that this is As Good As It Gets ™ as far as local changes to the request sender go.

Since the stability ship has sailed on the ringop interface wanting to return a response byte buffer from the forwarding path, the only other option would be to start adding a new faster forwarding path that doesn't have to read the response back.

For context, in production we're seeing that growing buffer space for ringpop forwarding dominates lifetime allocations (under the prior abstracted ioutil.ReadAll that the tchannel helper code hides).

jcorbin commented 6 years ago

Derp, I'd missed that io.Copy hides a read buffer allocation; updated to avoid even playing that game (I mean sure, we could've used io.CopyBuffer, but seemed better to just use bytes.Buffer.ReadFrom instead).

CLAassistant commented 4 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Joshua T Corbin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.