Skip to content
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

Support increment/decrement #29

Open
adzap opened this issue Aug 1, 2023 · 15 comments
Open

Support increment/decrement #29

adzap opened this issue Aug 1, 2023 · 15 comments

Comments

@adzap
Copy link

adzap commented Aug 1, 2023

Any reason not to support these cache methods? I gather you don't use them so you left them out.

Wondering if you want a PR for it?

@skatkov
Copy link
Contributor

skatkov commented Aug 25, 2023

I'm not a maintainer of this gem. but increment/decrement seems like standard interface for rails cache storage. It would be great to have it, just in case.

@dim
Copy link
Member

dim commented Aug 25, 2023

PR would be nice :)

@adzap
Copy link
Author

adzap commented Aug 27, 2023

Here is the solution I've been using. Not efficient as I'd like, a read and a write, which could be done with an upsert with conflict adding old and new value.

# Increment an integer value in the cache.
def increment(name, amount = 1, options = nil)
  options = merged_options(options)
  entry = read_entry(name) || Entry.new(options.fetch(:initial, 0))
  new_entry = Entry.new(entry.value.to_i + amount)
  write_entry(name, new_entry, **options)
  new_entry.value
end

# Decrement an integer value in the cache.
def decrement(name, amount = 1, options = nil)
  options = merged_options(options)
  entry = read_entry(name) || Entry.new(options.fetch(:initial, 0))
  new_entry = Entry.new(entry.value.to_i - amount)
  write_entry(name, entry, **options)
  new_entry.value
end

@dim
Copy link
Member

dim commented Aug 27, 2023

That's prone to races, I'm afraid. All DBs support atomic increments.

@skatkov
Copy link
Contributor

skatkov commented Aug 27, 2023

AR comes with #increment_counter and #decrement_counter functions, the only problem with those is that they only offer a way to increment and decrement by the value of one.

#update_counters seems like a generic implementation for updating counters, that could be used here.

@adzap
Copy link
Author

adzap commented Aug 28, 2023

Yeah it's not great. But anything more needed a bit more plumbing. Ideally we need add an upsert-like atomic increment which uses a raw (non-Marshaled) value and inserts the delta or adds the the delta to existing value on conflict.

ActiveRecord has upsert in v6.0 and 6.1 but there is no on_duplicate option, which v7.0 has, to set how you handle conflicts. Which leaves us needing to use raw SQL as far as I can tell.

Unless there are other ideas.

@adzap
Copy link
Author

adzap commented Aug 28, 2023

The existing AR counter methods assume a record exists, which we cannot.

@dim
Copy link
Member

dim commented Aug 28, 2023

We could use on_duplicate but I assume incrementing a Marshal.dump value with pure SQL could be difficult. Transactions maybe?

@adzap
Copy link
Author

adzap commented Aug 28, 2023 via email

@adzap
Copy link
Author

adzap commented Aug 29, 2023

Here's a quick hack for what I'm thinking

key = 'test'
value = 1

binary_key   = @model.connection.escape_bytea(key)
binary_value = @model.connection.escape_bytea(value.to_s)

str  = <<~SQL
INSERT INTO "activesupport_cache_entries"
    ("key","value","version","expires_at","created_at")
VALUES
    ('%{key}', '%{value}', '%{version}', '%{expires_at}', '%{created_at}')
ON CONFLICT ("key")
DO 
  UPDATE SET "value"=decode((encode(activesupport_cache_entries.value, 'escape')::int + encode(excluded."value", 'escape')::int)::text, 'escape'), "version"=excluded."version","expires_at"=excluded."expires_at","created_at"=excluded."created_at"
RETURNING
  value
SQL

res = @model.connection.execute(str % { key: binary_key, value: binary_value, version: 1, expires_at: 10.minutes.from_now.to_s(:db), created_at: Time.current.to_s(:db) })
@model.connection.unescape_bytea(res.entries[0]['value']).to_i

@adzap
Copy link
Author

adzap commented Aug 29, 2023

The expiry is not honoured so that would need fixing.

@skatkov
Copy link
Contributor

skatkov commented Aug 29, 2023

We can't use on_duplicate if you want to support < AR v7 though.

It's not yet released, but support for Rails 6 has been removed.
#35

@adzap
Copy link
Author

adzap commented Aug 29, 2023

Oh right. Well I need this for Rails 6.0 and 6.1 projects which are not going to be upgraded for a little while.

I'll go my own way I guess.

@dim
Copy link
Member

dim commented Aug 29, 2023

Ooops, my bad, I assumed Rails 6.1 is now reached EOL too, but it's only 6.0. Will bring it back in a separate PR - https://endoflife.date/rails

@dim
Copy link
Member

dim commented Aug 29, 2023

Ref: #37

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants