Skip to content

poll() only steps over one lost event per call, so a run of them never drains #130

Description

@TheophileDiot

poll() recovers from a lost event one index per call, so a run of lost events costs the full timeout on every call and a busy writer stays ahead of it forever. #87 is the same wedge, and the fix it got only covers the worker that starts at index 0.

broadcast() consumes an index before it stores anything, so a hole is left by two different failures, not one:

-- lib/resty/mlcache/ipc.lua
local idx, err = self.dict:incr(INDEX_KEY, 1, 0)
...
local ok, err, forcible = self.dict:set(idx, marshalled_event)
if not ok then
    return nil, "failed to insert event in shm: " .. err
end

if forcible then
    ...

A set() that returns no memory returns at that first branch, before the FORCIBLE_KEY block ever runs. So the flag #88 and #97 rely on is never set for that failure, and the index is gone all the same. That's the failure #129 reports, and it's the one we hit.

A worker past index 0 has no recovery path at all. poll() sets self.idx = idx and returns nil, "timeout", so it advances by a single index per call after burning the whole 0.3s. Under mass eviction that run of holes is thousands long, and the caller pays 0.3s each time.

We call update() on every HTTP request, so on our side this reads as every request taking 300ms and never recovering until the shm is emptied:

[BLACKLIST] error while adding element to cache : could not broadcast update: failed to insert event in shm: no memory
[ACCESS] can't update cachestore : could not poll ipc events: timeout

Measured on a model of the shdict, a run of 100 lost events takes 101 polls and 30s of timeout to drain today, against 2 polls and 0.3s when poll() walks to the next surviving index instead.

What I'd propose is that walk, capped so one call can't scan an unbounded range, still returning nil, "timeout" so the caller learns events were lost. It reads no flag, so the evicted event and the no memory hole are covered by the same rule, and events after the hole are still delivered. PR coming right behind this.

Is a bounded scan the shape you'd want, or would you rather give events an exptime and let the index sparsify on purpose?

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions