Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memcache binary protocol support #6

Open
renchap opened this issue Oct 16, 2014 · 25 comments
Open

Memcache binary protocol support #6

renchap opened this issue Oct 16, 2014 · 25 comments

Comments

@renchap
Copy link

renchap commented Oct 16, 2014

Hi,

mcrouter currently only supports ascii protocol, which is not ideal for performance. Is it planned to implement support for memcache binary protocol ? It would be great and would allow mcrouter to work with Dalli, the reference ruby gem to work with memcache.

I see that you support the umbrella protocol and are using it at Facebook, but I cant find any documentation on it. Is it for your internal use only ?

Thanks !

@alikhtarov
Copy link
Contributor

@renchap yes, the Umbrella protocol is something we use internally at Facebook. We don't want to document it in detail, since it probably won't be useful outside of Facebook, and we also want to have flexibility to change the format.
I like the idea of supporting the open source binary protocol, and it wouldn't be too difficult to implement. It would be ideal if we got a pull request for implementing this :)

@liyufu
Copy link

liyufu commented Mar 7, 2015

if the mcrouter currently supports the binary protocol now?
@renchap
if the binary protocol have a good performance to the ascii protocol?

@liyufu
Copy link

liyufu commented Mar 7, 2015

@renchap

if the binary protocol have a good performance to the ascii protocol?

@ryanmce
Copy link

ryanmce commented Mar 7, 2015

mcrouter does not currently support the memcached binary protocol. It does have a mcrouter-to-mcrouter binary protocol called umbrella, but memcached doesn't know how to parse it, so it's only useful in routing networks.

On Mar 7, 2015, at 3:03 AM, liyufu notifications@github.com wrote:

@renchap

if the binary protocol have a good performance to the ascii protocol?


Reply to this email directly or view it on GitHub.

@amckinley
Copy link

Any updates on this? We need binary protocol support before we can use mcrouter.

@alikhtarov
Copy link
Contributor

@amckinley: hi, no updates at this time. By the way, could you share how the binary protocol helps in your case? We're actually evaluating whether to migrate from ASCII to binary internally for memcached - some things we can think of is slightly better parsing performance (due to known sizes), and reply/request matching (less parsing bugs).

@cce
Copy link

cce commented Apr 25, 2015

We use the binary protocol because it seemed easier than worrying about ASCII-encoding binary keys like hashes or UTF-8 strings, and seemed faster as well for the reasons you mentioned. Would love to see support in mcrouter!

@ghost
Copy link

ghost commented Aug 4, 2015

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

@lcacciagioni
Copy link

Is this issue closed? Binary protocol support will be great

@erulabs
Copy link

erulabs commented Jul 13, 2016

It looks like PHP7 uses binary protocol for the memcached session handler by default these days. That means you get a pretty ugly error like:

Fatal Error: session_start(): Failed to create session ID: memcached

Since mcrouter doesn't support binary protocol, you'll need to tell PHP not to use it for sessions:

memcached.sess_binary_protocol = 0

So I'm betting this thread will get more and more visitors as people try to put PHP7 and Mcrouter together for the first time.

@nichochar
Copy link

Anyone with C++ skills want to tackle this? Maybe we can get some precisions on where in the codebase this change would happen, to make it easier for a contributor to start attacking the problem.

@mikeKBB
Copy link

mikeKBB commented Nov 4, 2016

we currently use the binary protocol with the enyim client, and we were hoping to plug in mcrouter to enable the replication and warm up features. But the ascii only support seems to be a major impediment.

we installed a mcrouter docker container, but on the initial tests the ascii items are timing out. is there some client tweak recommended or required other than specifying protocol is text?

@barryhatfield
Copy link

+1

@barryhatfield
Copy link

I was poking around in the latest version of the code (by no means am I an expert) and noticed some binary support stuff. Any update?

@jmswen
Copy link
Member

jmswen commented Feb 18, 2017

@barryhatfield, mcrouter supports a binary protocol (called Caret, if you see that word in our codebase) that has nothing to do with open-source memcached's binary protocol.

Unfortunately, at this point, we have no plans to add support for open-source memcached's binary protocol ourselves since we're happy with using Caret and ASCII internally. But, we would be willing to help guide anyone who wanted to contribute pull requests for open-source binary support.

@Ngo-The-Trung
Copy link

I'd like to take on this task

@andreazevedo
Copy link
Contributor

@Ngo-The-Trung great!

Let us know if you need anything, we will be happy to help.

@mujtabaahmed1991
Copy link

I am also interested to work on this, @Ngo-The-Trung lets get in touch.

@ELD
Copy link

ELD commented Jan 3, 2019

@Ngo-The-Trung Are you still working on this? If not, I'd be happy to take this track of work up. I'd really like to see mcrouter support the binary memcached protocol, too.

@Ngo-The-Trung
Copy link

@ELD please take over

@ELD
Copy link

ELD commented Jan 3, 2019

@Ngo-The-Trung Thank you, will do.

@ndobromirov
Copy link

From experience the binary protocol allows for much higher performance (overall) compared to plain text one, mainly because you can then use compression on the data and keys you set in memcacheD instances.

From there you are greatly reducing:

  1. The cache storage needs of the system. As a plus-side, you can cache more on the same servers or cache the same with less, saving either further CPU cycles on application layer or some more money in the pocket.
  2. The network transfer times for cache reads, as the data is compressed and not a in plain text format it is usually around 10 times smaller (or more). This should improve network related delays and related bottlenecks.

The only problem is that client side will have some more overhead in CPU to consume the data, but that is usually negligible compared to the network delays to fetch it. Thus having compression for memcache is a net-positive performance-wise.

I am a huge +1 on having binary protocol supported in mcrouter. This will allow us to test and experiment with all of it's features. As it is at the moment, we can not use this software, as we depend on compression for our servers to run normally.

@codyja
Copy link

codyja commented Jan 8, 2020

Would love to see Binary protocol supported as well. We use it for performance reasons too.

@darjisanket
Copy link

Any updates on support for binary protocol.

@ndobromirov
Copy link

As they've mentioned mcrouter does support binary protocol for transferring data.
This will reduce the network overhead in a case where you have 2-step mcrouter setup like:
box 1: web app + mcrouter1.
Box 2: Memcache + mcrouter2

The communication channel would be like:

  • web app -> mcrouter1 (text, unix socket)
  • mcrouter1 -> mcrouter2 (binary, non-standard, network)
  • mcrouter2 -> memcached (text, unix socket)

I have not tested this but in reality it should reduce network overheads (at least) and allow for all the features to be used.
Will it be faster will likely depend on the workload.

facebook-github-bot pushed a commit that referenced this issue May 26, 2021
…d up build.

Summary:
Same goal and idea as D26886048 (a3d9640) but a different implementation. This diff explicitly instantiates the templates rather than splitting the logic into multiple files.

## `TaoProtocolRouterInfo.cpp`

This file is currently the #2 entry in [Buck Critical Path Rules](https://fburl.com/scuba/buck_critical_path_rules/edmgxdyj) and #3 in [Buck Slow Targets](https://fburl.com/scuba/buck_slow_targets/f40w9jgc) Scuba tables, having taken about ~272 hours in the critical path of builds on devservers in the past 30 days.

This diff reduces the compile time from roughly 3:59 to 1:39 min, or **59% less** than before on a devbig.

## `MCTaoHelper.cpp`

This file is currently the #6 entry in [Buck Critical Path Rules](https://fburl.com/scuba/buck_critical_path_rules/edmgxdyj) and #5 in [Buck Slow Targets](https://fburl.com/scuba/buck_slow_targets/f40w9jgc)
 Scuba tables, having taken about ~72 hours in the critical path of builds on devservers in the past 30 days.

This diff reduces the compile time from roughly 3:42 to 2:07 min, or **43% less** than before on a devbig.

## `MemcacheRouterInfo.cpp`

This file is currently #7 in [Buck Slow Targets](https://fburl.com/scuba/buck_slow_targets/f40w9jgc) Scuba table.

This diff reduces the compile time from roughly 2:00 min to 57 sec, or **53% less** than before on a devbig.

 ---

## Build speed wins simulator results by antonl

```
mcrouter_wins_week_of_2021-05-25: Would have saved 106.4h over 7.0 days (or 2.88 HC saving) across 4683 builds
```
[Simulation Breakdown](https://www.internalfb.com/intern/everpaste/?color=0&handle=GKV8CwpQMQjnZKUBAFknDuHVUXZMbjEQAAAz)
```
mcrouter_wins_week_of_2021-05-18: Would have saved 112.1h over 7.0 days (or 3.04 HC saving) across 4707 builds
```
[Simulation Breakdown](https://www.internalfb.com/intern/everpaste/?color=0&handle=GLvbqAlHtMPnGVkPAKEEzK6Ex28bbjEQAAAz)
```
mcrouter_wins_week_of_2021-05-11: Would have saved 115.0h over 7.0 days (or 3.11 HC saving) across 4743 builds
```
[Simulation Breakdown](https://www.internalfb.com/intern/everpaste/?color=0&handle=GJk_LwtElCRy4oUAADGbmbyFqkkBbjEQAAAz)
```
mcrouter_wins_week_of_2021-05-04: Would have saved 101.4h over 7.0 days (or 2.75 HC saving) across 4329 builds
```
[Simulation Breakdown](https://www.internalfb.com/intern/everpaste/?color=0&handle=GM6UaQmVCQap-twBAAdSPmjZm5NpbjEQAAAz)

Reviewed By: stuclar

Differential Revision: D28629070

fbshipit-source-id: 35ae6a34c158ddd33b4a9764076d8d172d81ebbd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests