Skip to content

Commit

Permalink
Remove delta comparison for fillup
Browse files Browse the repository at this point in the history
It is better to let the DB compare the level to the
capacity (using it's own float representation) and to
return based on that. Since we use LEAST the value would be
equal (exactly) to capacity at overflow.

Closes #7
  • Loading branch information
julik committed Nov 28, 2023
1 parent 1c2cd7a commit 2b325c7
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions lib/pecorino/leaky_bucket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,19 @@ def add_tokens(n_tokens)
t.level + :fillup - (EXTRACT(EPOCH FROM (EXCLUDED.last_touched_at - t.last_touched_at)) * :leak_rate)
)
)
RETURNING level
RETURNING
level,
-- Compare level to the capacity inside the DB so that we won't have rounding issues
level >= :capa AS did_fillup
SQL

# Note the use of .uncached here. The AR query cache will actually see our
# query as a repeat (since we use "select_value" for the RETURNING bit) and will not call into Postgres
# query as a repeat (since we use "select_one" for the RETURNING bit) and will not call into Postgres
# correctly, thus the clock_timestamp() value would be frozen between calls. We don't want that here.
# See https://stackoverflow.com/questions/73184531/why-would-postgres-clock-timestamp-freeze-inside-a-rails-unit-test
level_after_fillup = conn.uncached { conn.select_value(sql) }
upserted = conn.uncached { conn.select_one(sql) }
capped_level_after_fillup, did_fillup = upserted.fetch("level"), upserted.fetch("did_fillup")

State.new(level_after_fillup, (@capacity - level_after_fillup).abs < 0.01)
State.new(capped_level_after_fillup, did_fillup)
end
end

0 comments on commit 2b325c7

Please sign in to comment.