Document default options set by PyMemcacheCache.

[yakirsudry]

I'm not sure why would we do that, since changing to True will slightly speed up the usage of the cache

[Simon Charette]

​Parallel conversation on Github. Nick mentioned that it was done to make the backend a drop in replacement for other MC backends but I'm curious to see if any tests fail without this override. At the least we should mention in the docs that these defaults are set as django-pymemcache or other pymemcache based backend don't behave this way and switching to the backend provided by Django might cause a change in behaviour.

[Nick Pope]

Thank you for dredging up my comment Simon.

As mentioned there, I was trying to ensure that the new pymemcache backend was compatible with the expectations of Django by ​setting various options.

If I remove 'default_noreply': False, then a heap of tests fail because pymemcache always returns True for some operations:

$ python runtests.py --settings=test_sqlite --parallel=1 --timing cache.tests.PyMemcacheCacheTests
Testing against Django installed in '/home/pope1ni/Sources/django/django'
Found 58 tests.
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
F....F..F......ss.........F..F........................F.s.
======================================================================
FAIL: test_add (cache.tests.PyMemcacheCacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pope1ni/Sources/django/tests/cache/tests.py", line 306, in test_add
    self.assertIs(cache.add("addkey1", "newvalue"), False)
AssertionError: True is not False

======================================================================
FAIL: test_cache_versioning_add (cache.tests.PyMemcacheCacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pope1ni/Sources/django/tests/cache/tests.py", line 767, in test_cache_versioning_add
    self.assertIs(cache.add('answer1', 37, version=2), False)
AssertionError: True is not False

======================================================================
FAIL: test_cache_versioning_get_set_many (cache.tests.PyMemcacheCacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pope1ni/Sources/django/tests/cache/tests.py", line 900, in test_cache_versioning_get_set_many
    self.assertEqual(cache.get_many(['ford3', 'arthur3'], version=2), {'ford3': 37, 'arthur3': 42})
AssertionError: {'ford3': 37} != {'ford3': 37, 'arthur3': 42}
- {'ford3': 37}
+ {'arthur3': 42, 'ford3': 37}

======================================================================
FAIL: test_delete_nonexistent (cache.tests.PyMemcacheCacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pope1ni/Sources/django/tests/cache/tests.py", line 344, in test_delete_nonexistent
    self.assertIs(cache.delete('nonexistent_key'), False)
AssertionError: True is not False

======================================================================
FAIL: test_forever_timeout (cache.tests.PyMemcacheCacheTests)
Passing in None into timeout results in a value that is cached forever
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pope1ni/Sources/django/tests/cache/tests.py", line 597, in test_forever_timeout
    self.assertIs(cache.add('key1', 'new eggs', None), False)
AssertionError: True is not False

======================================================================
FAIL: test_touch (cache.tests.PyMemcacheCacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pope1ni/Sources/django/tests/cache/tests.py", line 484, in test_touch
    self.assertIs(cache.touch('nonexistent'), False)
AssertionError: True is not False

----------------------------------------------------------------------
Ran 58 tests in 12.032s

FAILED (failures=6, skipped=3)
Destroying test database for alias 'default'...
Total database setup took 0.090s
  Creating 'default' took 0.090s
Total database teardown took 0.000s
Total run took 12.187s

It is perfectly possible for developers to add 'default_noreply': True, to their OPTIONS for the cache to override this.

I find it a strange default. Yes, it is faster, but probably less safe - see the failure for test_cache_versioning_get_set_many above.

We can add an admonition to state what options are being set by default for pymemcache, but I don't feel that we need to change the behaviour.

[Simon Charette]

Thanks for the extra details Nick.

In the light of these failures I think the only way we could get tests passing/ensure the backend fully functional without turning reply on per client would be to adjust all the methods that require a reply from MC to work properly to explicitly pass noreply=False but I'm not sure there would be much benefit from that.

Documenting to enable the option as is would simply break the backend so I'm not convinced we should even do that anymore. Were you aware of that yakirsudry?

[Nishant Sagar]

I checked the pymemcached documentation and it says noreply will not read errors returned from the memcached server.

If a function with noreply=True causes an error on the server, it will still succeed and your next call which reads a response from memcached may fail unexpectedly.

pymemcached will try to catch and stop you from sending malformed inputs to memcached, but if you are having unexplained errors, setting noreply=False may help you troubleshoot the issue.

What do we supposed to do now?

PS- I'm new to this project so can anyone give me some suggestion to fix this issue

[Adam Johnson]

We can add an admonition to state what options are being set by default for pymemcache, but I don't feel that we need to change the behaviour.

+1 to this

adjust all the methods that require a reply from MC to work properly to explicitly pass noreply=False but I'm not sure there would be much benefit from that.

Whether the reply is required depends on context... e.g. in the context of tests, we check the status of operations, but user code may not care.

[Mariusz Felisiak]

​PR

[GitHub <noreply@…>]

In bb2c5f69:

Fixed #32749 -- Doc'd PyMemcacheCache defaults.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 72358f01:

[4.1.x] Fixed #32749 -- Doc'd PyMemcacheCache defaults.

Backport of bb2c5f69f47466fa52f3cf2727d10b3ebd79a4da from main