[Feature Request] initial value for ActiveSupport::Cache::MemCacheStore#increment and decrement

Hi there.

I made a Pull Request to fix bug on ActiveSupport::Cache::MemCacheStore#increment

and decrement for initialization of not stored value.

https://github.com/rails/rails/pull/30716

Regarding initialization, the current document says as follows.


# Increment a cached value. This method uses the memcached incr atomic

# operator and can only be used on values written with the :raw option.

# Calling it on a value not stored with :raw will initialize that value

# to zero.

However, I would rather initialize the value with amount for increment and - amount for decrement

as if the not stored value is initialized to 0 prior to the specified operation.


# Before

@data.incr(normalize_key(name, options), amount, options[:expires_in], 0)

@data.decr(normalize_key(name, options), amount, options[:expires_in], 0)

# After

@data.incr(normalize_key(name, options), amount, options[:expires_in], amount)

@data.decr(normalize_key(name, options), amount, options[:expires_in], -amount)

The reason is that we want to implement appropriate lazy initialization.

For example, simple counter with lazy initialization can be written with instance variable as follows.


def increment(amount)

@counter ||= 0

@counter += amount

end

If we implement the above counter with current ActiveSupport::Cache::MemCacheStore, it can be written as follows.


def increment(amount)

result = @cache.increment(amount)

result != 0 ? result : @cache.increment(amount)

end

As you can see, the initialization to 0 inside @cache actually does not help us write lazy initialization.

We should still handle 0 instead of nil. What is worse, if we use increment and decrement concurrently

on the same key, there is no way to calculate the accurate count.

With the proposed feature request the above code can be simplified as follows.


def increment(amount)

@cache.increment(amount)

end

Also, I have found the same specification on incr of Redis.

If the idea is O.K., I will submit another PR or change the current PR to address the issue.

IMHO, as this initialization has been disabled for a long time, it is about time to improve the specification.

+1 to matching Redis-style INCR. Returning nil is useful in some cases, like lazy-initializing a counter to a value that’s expensive to calculate, but for typical use it’s a strange hassle.

Note that Memcached doesn’t support negative counters, so @data.decr(normalize_key(name, options), amount, options[:expires_in], -amount) may have unexpected results (setting to 0).