Skip to content

Added support for GAT and GATS (Get and Touch) memcached commands.#270

Closed
sergmour wants to merge 3 commits into
facebook:masterfrom
sergmour:gat_branch
Closed

Added support for GAT and GATS (Get and Touch) memcached commands.#270
sergmour wants to merge 3 commits into
facebook:masterfrom
sergmour:gat_branch

Conversation

@sergmour

Copy link
Copy Markdown

Summary:
This change addresses issue #222: Feature Request: Support GAT & GATS (Get And Touch Commands). It follows the change in the memcached ASCII protocol as of Oct 2017: GAT and GATS (Get and Touch) commands were added to memcached ASCII protocol.

  • GAT/GATS commands produce same output as GET/GETS commands;
  • GAT/GATS commands are included in GetLike routing group and are thus routed same way as GET/GETS commands;
  • Touch portion of GAT/GATS commands will follow the routing logic:
    • For parallel routes like AllAsyncRoute-AllSyncRoute all replicas are touched;
    • For serial routes like Failover/MissFailover only one replica is touched.
  • Added basic tests for GAT/GATS commands.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Summary:
This change addresses issue facebook#222: Feature Request: Support GAT & GATS (Get And Touch Commands). It follows the change in the memcached ASCII protocol as of Oct 2017: GAT and GATS (Get and Touch) commands were added to memcached ASCII protocol.

* GAT/GATS commands produce same output as GET/GETS commands;
* GAT/GATS commands are included in GetLike routing group and are thus routed same way as GET/GETS commands;
* Touch portion of GAT/GATS commands will follow the routing logic:
	- For parallel routes like AllAsyncRoute-AllSyncRoute all replicas are touched;
	- For serial routes like Failover/MissFailover only one replica is touched.
* Added basic tests for GAT/GATS commands.
@andreazevedo

Copy link
Copy Markdown
Contributor

Thanks for working on this @sergmour!
I'll pull it in and run some internal tests/review and will get back to you soon.

@facebook-github-bot facebook-github-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreazevedo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@sergmour

sergmour commented Dec 6, 2018

Copy link
Copy Markdown
Author

@andreazevedo : Thanks Andre! It'll be really helpful; we have customers waiting on that feature.

@sergmour

Copy link
Copy Markdown
Author

@andreazevedo : Hi Andre, what's the status of this PR? Is anyone looking into it? What's the status of the internal tests/review?
Thanks!

@andreazevedo

Copy link
Copy Markdown
Contributor

Sorry about that @sergmour.
I was working on it, but got side tracked by end-of-year activities.
I'll get back to it next Monday!

@andreazevedo

Copy link
Copy Markdown
Contributor

Hey @sergmour,

I just wanted to provide some feedback.
We will merge your change in the next few days (probably tomorrow). I finished reviewing the code internally. Everything looks great, thanks for working on it!
I made some tiny adjustments on your behalf - it would be easier than going over a round of comments/updates. I hope you don't mind.
It took sometime because, for feature parity, I actually had to implement get and touch on our fork of memcached that we use internally here at Facebook.

Thank you again for working on this!

@sergmour

Copy link
Copy Markdown
Author

Thanks Andre! I definitely wouldn't mind any adjustments, thanks for this. And hopefully we'd be adding more stuff to mcrouter in the near future.

@andreazevedo

Copy link
Copy Markdown
Contributor

This is now available on trunk! It will also be available in version 41.
Version 40 is currently being tested and rolled out internally by @andreimaximov, and it will published here soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants