Skip to content

Conversation

@taoky
Copy link

@taoky taoky commented Nov 30, 2024

When debugging the conn limiter today, I tried with this code in development env, to help me get conn counter value in HTTP header:

local delay, err = limit_traffic.combine(limiters, keys, states)
if not delay then
    -- ...
else
    for i, limiter in ipairs(limiters) do
        local delay, err = limiter:incoming(keys[i], false)
        local limiter_name = limiters_name[i]
        ngx.header[ "X-RateLimit-" .. limiter_name ] = err
        ngx.header[ "X-RateLimit-" .. limiter_name .. "-Key" ] = keys[i]
    end
end

for i, limiter in ipairs(limiters) do
    if limiter.is_committed then
        if limiter:is_committed() then
            ngx.log(ngx.WARN, limiters_name[i])
            table.insert(limiters_to_leave, limiter)
            table.insert(keys_to_leave , keys[i])
        end
    end
end

And surprised by that, after I added the limiter:incoming(keys[i], false) here, the limiter:is_committed() returns false even when it is actually committed.

In conn.md, it says:

* `commit` is a boolean value. If set to `true`, the object will actually record the event
in the shm zone backing the current object; otherwise it would just be a "dry run" (which is the default).

But it is actually not just "dry run" -- incoming would always set committed to false at first not matter what.

function _M.incoming(self, key, commit)
    local dict = self.dict
    local max = self.max

    self.committed = false
    -- ...
end

This PR tries to fix the lua code to align with docs, and adds a testcase for that.

committed: true
6: 1, conn: 3
committed: false
committed: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I doubt if this change makes sense for is_committed() docs:

- Returns `true` if the previous [incoming](#incoming) call actually commits the event
+ Returns `true` if the previous [incoming](#incoming) calls actually have committed the event

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

Successfully merging this pull request may close these issues.

2 participants