cachex icon indicating copy to clipboard operation
cachex copied to clipboard

Clarification regarding setting TTL under [successful] transactions

Open cognivore opened this issue 4 years ago • 4 comments

I'm using Cachex for in-BEAM anti-spam. When I had the following code:

    {:ok, %{accept: accept, front: front} =
      Cachex.transaction(:nimrod, [ip], fn sentinel ->
        {:ok, front} = Cachex.get(sentinel, ip)
        
        accept =
          case front do
            nil ->
              Cachex.put(sentinel, ip, next_second())
              true
            {_, next_allowed} ->
              tau0 >= next_allowed
          end

        front1 =
          if accept do
            front1 = next_second()
            Cachex.update(sentinel, ip, front1)
            front1
          else
            front1 = {_, tau1} = exponential_rate_limiter(front)
            Cachex.update(sentinel, ip, front1)
            Cachex.expire_at(sentinel, ip, 1000 * (tau1 |> DateTime.to_unix()) + 1000))
            front1
          end
          
        %{accept: accept, front: front1}
      end)
      
    if accept do
      handle(conn)
    else
      reject(conn, front)
    end

despite me running expire_at, after ~3 seconds (I think, it's default Janitor TTL), my caches were expiring and a spammer could break through again. Then I thought of trying to set TTL outside of transaction, like so:

    {:ok, %{accept: accept, front: front, expire_at: expire_at}} =
      Cachex.transaction(:nimrod, [ip], fn sentinel ->
        {:ok, front} = Cachex.get(sentinel, ip)

        accept =
          case front do
            nil ->
              Cachex.put(sentinel, ip, next_second(), ttl: 2000)
              true

            {_, next_allowed} ->
              tau0 >= next_allowed
          end

        front1 =
          {_, tau1} =
          if accept do
            front1 = next_second()
            Cachex.update(sentinel, ip, front1)
            front1
          else
            Logger.warn("Spam from #{ip |> String.slice(0..7)} @ #{inspect(front)}")
            front1 = exponential_rate_limiter(front)
            Cachex.update(sentinel, ip, front1)
            front1
          end

        %{accept: accept, front: front1, expire_at: 1000 * (tau1 |> DateTime.to_unix()) + 1000}
      end)

    if accept do
      handle(conn)
    else
      Cachex.expire_at(:nimrod, ip, expire_at)
      reject(conn, front)
    end

and TTLs are handled correctly now.

I think, it would be useful to clarify this behaviour in TTL sections of docs (perhaps I have missed these clarifications). Conversely, if the first snippet should work in principle, I'll refactor my code back to whence it came, and will try to make it work / test more carefully.

cognivore avatar Jul 31 '21 01:07 cognivore

Hi @cognivore!

I'm super late to this, my bad, but I think this is actually a bug? I'm not sure because I'm not totally following the first snippet at a glance. Is there a smaller case you can demonstrate this with?

I believe the expire call should function just fine from inside a transaction, I can't imagine why not - if you can get me a smaller reproduction case, I'll definitely take a look at it!

whitfin avatar May 31 '22 01:05 whitfin

Alright, will do my best to make a minimum test case!

And absolutely no problem for being "super late", people benefiting from FOSS shouldn't expect tech support :) ♥

cognivore avatar May 31 '22 01:05 cognivore

Hey @cognivore, I just wanted to follow up on this if you had anything else to provide or anything you might need changed in Cachex to resolve this?

If not, I can go through some tests for this myself - I'm going to bump some dependencies and stuff so I'll fix anything we find here if there's anything to change.

whitfin avatar Nov 23 '22 00:11 whitfin

@whitfin it's funny! I didn't touch that code since then, but I'll do a maintenance pass on the repo where I use Cachex (adding Bamboo.SMTP stuff), and will make a minimal test case in the coming weeks.

If you have some time to dig my spaghetti, this commit rolls back the code from the issue:

https://github.com/doma-engineering/contact_form/commit/1da9f84e53bad5c9a528c70898459d1d9d505716

This commit is how the code started:

https://github.com/doma-engineering/contact_form/commit/72aeb5af0fa099305f29618cc6df4a6e1360d84f

I'll try my best to make a minimal test case next week though, if not -- within two weeks. See if this timeline works for you.

Thank you for the follow-up!

cognivore avatar Nov 25 '22 16:11 cognivore