Skip to content

Do not validate return code if set was successful#26816

Merged
butonic merged 1 commit into
masterfrom
fix-25692
Dec 15, 2016
Merged

Do not validate return code if set was successful#26816
butonic merged 1 commit into
masterfrom
fix-25692

Conversation

@VicDeo

@VicDeo VicDeo commented Dec 12, 2016

Copy link
Copy Markdown
Contributor

Description

Do not call verifyReturnCode for successful set operation.

Related Issue

Fixes #25692

Motivation and Context

According to http://php.net/manual/en/memcached.set.php Memcached::set has a boolean result,
but it can return false both for unsuccessful operation and when it is called with false as a value http://php.net/manual/en/memcached.set.php#92259

So there is no need to validate Memcached::set when it returns true.
Sometimes it misbehaves as in #25692

How Has This Been Tested?

  1. install ownCloud 9.1.x ( into the following envronment:
    PHP 5.5.9 + php-memcached extension + memcached server

  2. Add to config.php:

'memcache.local' => '\\OC\\Memcache\\APCu',
 'memcache.distributed' => '\\OC\\Memcache\\Memcached', 'memcached_servers' => array ( 0 => array ( 0 => 'localhost', 1 => 11211, ), ),
  1. Perform an integrity check

  2. Perform an integrity check once again

  3. See "Exception":"Exception","Message":"Error 21 interacting with memcached : SERVER END","Code":0

  4. Repeat 4 & 5 Several times

  5. Apply changes from this PR

  6. Repeat 4. and see that 5. does not happen any more

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@VicDeo VicDeo added this to the 10.0 milestone Dec 12, 2016
@mention-bot

Copy link
Copy Markdown

@VicDeo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975, @butonic and @bantu to be potential reviewers.

@VicDeo

VicDeo commented Dec 12, 2016

Copy link
Copy Markdown
Contributor Author

@PVince81 IMO backport is required as it affects upgrade

@VicDeo VicDeo changed the title Do not validate return code is set is successful Do not validate return code if set was successful Dec 12, 2016
@VicDeo

VicDeo commented Dec 12, 2016

Copy link
Copy Markdown
Contributor Author

Jenkins's dead:
error: index-pack died of signal 15
fatal: index-pack failed

@PVince81

Copy link
Copy Markdown
Contributor

Makes sense 👍

You might need to rebase to revive Jenkins.

@VicDeo

VicDeo commented Dec 15, 2016

Copy link
Copy Markdown
Contributor Author

@PVince81 rebased, tests are passed

@butonic

butonic commented Dec 15, 2016

Copy link
Copy Markdown
Contributor

👍 nice find @VicDeo

@VicDeo

VicDeo commented Dec 15, 2016

Copy link
Copy Markdown
Contributor Author

Stable9.1 #26826
Stable9 #26827

@SergioBertolinSG

Copy link
Copy Markdown
Contributor

@VicDeo this can be tested using php5-apcu version 4.0.2 or it requires a newer one?

@SergioBertolinSG

Copy link
Copy Markdown
Contributor

Ok 4.0.2 cannot be used, using 4.0.7.

@VicDeo

VicDeo commented Jan 27, 2017

Copy link
Copy Markdown
Contributor Author

@SergioBertolinSG I don't remember exact version but apcu is unrelated to the issue anyway.

@lock

lock Bot commented Aug 4, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Aug 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memcache error when update to 9.1.0

5 participants