fix(introspection): coalesce concurrent cache misses

[shreemaan-abhishek]

Summary

• coalesce concurrent introspection cache misses for the same token within an NGINX instance
• share cacheable, non-cacheable, and failed introspection results with requests waiting on the same lock generation
• treat the per-token lock as a bounded lease that must outlive the introspection request
• recheck the cache and fall back to direct introspection when coordination times out or fails
• keep cache hits lock-free and preserve explicit introspection_cache_ignore behavior
• document the coordination options and cover concurrency, bypass, failure, timeout, and lease behavior

Closes #573.

Checklist

• Tests pass with docker build -f tests/Dockerfile . -t lua-resty-openidc/test
• Tests pass with docker run -t --rm lua-resty-openidc/test:latest
• ChangeLog is updated for behavior changes, bug fixes, features, or user-visible documentation changes

[shreemaan-abhishek]

@kevinlzw could you please review this PR?

[kevinlzw]

Sorry for late, please check below comments.

[kevinlzw]

The owner token does not make the release generation-safe because dict:get() and dict:delete() are separate shared-dict operations.
A possible interleaving is:

1. Worker A reads the lock and sees its own owner token.
2. A's lease expires and worker B acquires the same lock key for the next generation.
3. Worker A calls delete() and removes B's lock.
   A third worker can then acquire the lock and issue another introspection request while B is still in flight. ngx.shared.DICT does not provide an atomic compare-and-delete primitive, so I do not think this implementation can safely support an owner continuing after its lock lease has expired.
   Could we instead treat the lock as a lease, require introspection_lock_exptime to exceed the bounded introspection request duration, and avoid the “outlasted generation” contract? That would make the locking model much easier to reason about.

[kevinlzw]

I think cache-miss coalescing should remain a transparent optimization. At the moment, if lock acquisition times out, this returns an error without attempting introspection. With the default five-second timeout, a temporarily slow IdP can therefore turn otherwise valid tokens into authentication failures for all waiting requests.
This changes the existing availability behavior: before this PR, those requests would each perform introspection and could still succeed.
Could lock timeout/failure re-check the cache and then fall back to the existing direct introspection path by default? A strict fail-closed mode could be added later as an explicit opt-in, but it should not be the default behavior.

[kevinlzw]

The generation-scoped temporary-result relay is doing a lot of work to share endpoint failures and non-cacheable responses. It adds a fairly complex state machine around a cache-miss optimization and also changes failure/retry behavior.
Would it be reasonable to scope the first version to cacheable successful results only? The standard flow would then be: cache lookup, acquire lease, cache lookup again, introspect once, populate the cache, release. For failed or non-cacheable responses, releasing the lease and preserving the existing direct-introspection behavior would be simpler and more backward-compatible.

[shreemaan-abhishek]

For the temporary-result relay, the motivating APISIX case is an overloaded introspection endpoint returning 503. Limiting coordination to cacheable successes means every waiter eventually retries introspection, so the overload scenario remains unresolved. Would you be open to retaining in-flight sharing for failed and non-cacheable results after the lease and fallback changes, or would you prefer this PR to address only cacheable successes and defer that behavior?

[kevinlzw]

Thanks for the update. The fallback change addresses my main compatibility concern. Your explanation makes sense: limiting coordination to cacheable successes would leave the original 503 overload scenario unresolved, so I’m happy to retain in-flight sharing for failed and non-cacheable results.

A few focused changes would still help:

• Let waiters consume the result of the generation they originally observed without first acquiring the lock or switching to a newer generation.
• Restore the owner check before releasing the lock as a defensive measure, while documenting that it is not atomic and does not fully eliminate the race.
• Add multi-worker tests covering lease expiry and coordination-state eviction. Duplicate upstream calls may occur when coordination breaks down, but coordination failure alone should not cause authentication failures.

I’m comfortable treating coalescing as a best-effort optimization with these limitations documented. Happy to discuss the trade-offs further.

[shreemaan-abhishek]

Implemented all three follow-ups in 7784c0e:

• Waiters retain the first lock generation they observe and check, in order, the regular introspection cache and that generation's temporary result before attempting to acquire the lock. They no longer switch to later observed generations or relay a completed result through a newly acquired generation.
• Lock release now checks that the shared-dict value still matches the caller's owner token before deleting it. The comment explicitly documents that get plus delete is not atomic and only narrows the race window.
• Added multi-worker coverage (four nginx workers) for both lease expiry and forced coordination-state eviction. The tests verify that multiple workers participate, duplicate upstream calls are tolerated after coordination breaks down, and every client request still receives the successful introspection result rather than an authentication failure.

Validation:

• Focused introspection suite: 93 successes, 0 failures
• Full suite: 597 successes, 0 failures, 0 errors, 1 pre-existing pending test

[shreemaan-abhishek]

@kevinlzw do you have any further comments?

[kevinlzw]

@shreemaan-abhishek
Sorry that I missed this, let me check again tonight.

[kevinlzw]

Thanks for addressing the follow-ups. The latest changes address my concerns within the agreed best-effort coalescing model, with direct introspection as the fallback when coordination fails. LGTM.

[kevinlzw]

Hi @shreemaan-abhishek ,

I need to get a new api key to release in luarocks, so please wait for sometime. Thank you!

[shreemaan-abhishek]

@kevinlzw thank you so much for the prompt responses and for the release plan too... I was hoping I would raise another PR to fix this issue: #574... could you plan the release after the second fix? Thank you so much, you are awesome.

[kevinlzw]

Hi @shreemaan-abhishek ,
Thank you. I will release this version and release another minor release after you raise another PR. Reason: I have already published the release in GitHub.