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

Use the native implementation for Memcached::decrement #4307

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@adsr
Contributor

adsr commented Nov 21, 2014

Calling Memcached::decrementByKey with an empty string causes all
decrement operations to hash to a single node in the server list. I suspect
the intention was to pass along a NULL, but calling the native implementation
like ::increment seems cleaner either way.

Use the native implementation for Memcached::decrement
Summary: Calling Memcached::decrementByKey with an empty string causes all
decrement operations to hash to a single node in the server list. I suspect
the intention was to pass along a NULL, but calling the native implementation
like ::increment seems cleaner either way.
@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented Nov 21, 2014

This pull request has been imported into Phabricator, and discussion and review of the diff will take place at https://reviews.facebook.net/D29391

@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented Nov 21, 2014

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 - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Nov 24, 2014

@adsr: We need the CLA completed before we can look at the change

@adsr

This comment has been minimized.

Contributor

adsr commented Nov 24, 2014

@fredemmott CLA should be in now. Thanks!

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Nov 24, 2014

Thanks - if you did it online and filled in the github username field, the automation should comment here shortly; otherwise, drop me a mail at fe@fb.com with a link to this PR, your real name (from the CLA), and I'll get it hooked up

@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented Nov 24, 2014

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

@fredemmott

This comment has been minimized.

Contributor

fredemmott commented Nov 24, 2014

Review of the change happens on that phabricator link - please reply to the comments there :)

@adsr adsr force-pushed the adsr:master branch from f78f34b to c0d2e6f Dec 1, 2014

Add test for previous Memcached::decrement fix
Summary: The test requires two memcached servers to be running to ensure the
decrement operation is hashing to the right server under the hood.

@hhvm-bot hhvm-bot closed this in e2ffb44 Dec 5, 2014

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