Opened 14 years ago

Closed 14 years ago

Last modified 13 years ago

#14299 closed (wontfix)

Add additional cache.*_many functions

Reported by: Michael Manfre Owned by: Michael Manfre
Component: Core (Cache system) Version: dev
Severity: Keywords: cache, add_many, offset_many
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

It would be helpful to have the cache framework allow backends to implement more bulk operations. Attached is a patch that adds support for add_many() and offset_many().

add_many()

To add multiple values more efficiently, use add_many() to pass a dictionary
of key-value pairs. add_many() returns a dictionary with all the keys passed
and their values being a bool indicating whether or not the value was added::

	>>> cache.set('a', 1)
	>>> cache.add_many({'a': 2, 'b': 1})
	{'a': False, 'b': 1}

offset_many()

To increment and decrement multiple values more efficiently, use
offset_many() to pass a dictionary of key-value pairs, where the value is
the delta you would like apply to the existing value. offset_many()
returns a dictionary with all the keys passed and their new value. If a key is
not found or there was an error, its value will be False::

	>>> cache.set_many({'a': 10, 'b': 10})
	>>> cache.offset_many({'a': 1, 'b': -1, 'c': 1})
	{'a': 11, 'b': 9, 'c': False}

Attached patch is against trunk, includes tests and docs.

Attachments (1)

django-cache-add_many.diff (7.9 KB ) - added by Michael Manfre 14 years ago.

Download all attachments as: .zip

Change History (7)

by Michael Manfre, 14 years ago

Attachment: django-cache-add_many.diff added

comment:1 by Michael Manfre, 14 years ago

Patch contains a fix for memcached's incr() and decr(), which raise the 'Key not found' ValueError if delta is negative. If needed, I can split that fix off to a separate ticket and change offset_many() to explicitly call either incr() or decr().

comment:2 by Jacob, 14 years ago

Resolution: wontfix
Status: newclosed

Django's cache API deliberately mimics memcached's API. Specifically, it doesn't implement any operations that aren't in memcached's API. Since these new _many operations aren't natively supported by memcached, they're not a good fit for Django's cache API.

If you're attached to 'em you can always write a custom cache backend and use that instead.

The incr/decr fixes should be filed a separate ticket; those are indeed bugs and should be fixed.

comment:3 by Michael Manfre, 14 years ago

For clarification, I'm assuming you mean the Django API loosely mimics the python-memcached client library, which exposes delete_multi and set_multi to reduce connection latency, and not the memcached protocol commands? Is this design restriction documented anywhere? Are there any other unwritten or hidden decisions along these lines regarding the cache framework?

comment:4 by Jeremy Dunck, 14 years ago

Patch needs improvement: set

Feedback as I come across things -- I somehow didn't notice Jacob's already wontfix'd this ticket. Perhaps feedback is useful anyway. I leave Jacob to answer your previous questions.

  1. Did you test against other backends? I see you've implemented in base, but it's not clear to me that base will work in all backends.
  2. Dummy .set_many should return the empty dict rather than None
  3. Why the change to smart_str in memcache incr/decr? Unrelated issue?
  4. Replace: "add_many() returns a dictionary with all the keys passed and their values being a bool indicating whether or not the value was added" with "add_many() returns a dictionary containing all keys passed in and boolean values indicating whether the value was added."
  5. test_add_many and test_offset_many are repeated in your diff. Please remove the dupes.
  6. move comment "# add_many takes a second timeout parameter" below the call to delete_many.
  7. In test_add_many, please ensure to test both cases where a key does exist and where it does not.
  8. In offset_many, I'm not sure about returning booleans for a failed incr/decr. Is this something you've seen in other backends?

comment:5 by Michael Manfre, 14 years ago

jdunck, thanks for the feedback. I'm not sure it's worth the time to update the patch given the wontfix.

  1. Tests in the base mixin run for all of the backends, except dummy.
  2. Agreed.
  3. Unrelated issue see #14315
  4. ok
  5. One set is for the dummy backend and the second set is for the rest of them.
  6. ok
  7. ok
  8. A more conforming approach would be to return a dict containing the new values. Any keys with errors would be omitted or set to None.

comment:6 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top