Skip to content

ngx_cache_purge: make the REFRESH page size configurable - #81

Open
yaoge123 wants to merge 1 commit into
nginx-modules:mainfrom
yaoge123:pr/refresh-dir-prune
Open

yaoge123 wants to merge 1 commit into
nginx-modules:mainfrom
yaoge123:pr/refresh-dir-prune

Conversation

@yaoge123

Copy link
Copy Markdown
Collaborator

Problem

A REFRESH page always restarts its scan at the cache root. The traversal state — open directory handles and the descent stack — cannot survive between requests, and readdir has no portable way to resume at an offset, so the cursor carries only a file hash and every page re-walks the pending directories.

On caches with hundreds of thousands of entries and page counts in the low thousands, that repeated traversal dominates the cost of a whole-prefix refresh: the work grows with entries multiplied by pages instead of entries alone.

Change

Add cache_purge_refresh_batch <number> (default 32, range 1..256) and size the page entries from it.

  • The bounded page remains: a request still collects at most one batch and still returns a cursor.
  • A larger batch means fewer pages for the same selector, and therefore fewer repeated walks.
  • Memory grows linearly with the batch, because every collected entry reserves a native key buffer, so the default stays conservative.

The entry array moves to the end of the scan structure and the structure is allocated with room for the configured capacity, so a page still needs one allocation.

Compatibility

The directive is new; the default reproduces the previous page size, so existing configurations behave identically. The cursor format, the response schema and the request contract are unchanged.

Tests

  • the directive is accepted in http, server and location contexts
  • zero and out-of-range values are rejected at configuration time
  • a REFRESH with a larger page size completes and keeps an unchanged entry
  • the full REFRESH suite still passes with the default
  • full suite: seven TAP suites pass, plus strict debug/release builds

Copilot AI balanced review requested due to automatic review settings October 28, 2026 00:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The runtime test scans only one entry, so it cannot detect the old 32-entry cap or verify pagination and allocation at the configured batch size of 128.

1 open issue
What changed in this PR

This PR makes REFRESH page capacity configurable, reducing repeated cache-directory scans for large refreshes while preserving the default page size and request/response contract.

Changes:

  • Adds cache_purge_refresh_batch with a default of 32 and a valid range of 1–256.
  • Sizes the scan’s entry storage and collection limit from the configured capacity.
  • Documents the memory trade-off and adds configuration and runtime tests.
File Description
t/​purge_runtime.t Adds runtime coverage for a larger configured batch.
t/​config.t Tests accepted values, range rejection, and server-level configuration.
ngx_cache_purge.h Defines the maximum batch size and configuration field.
ngx_cache_purge_refresh.c Allocates and collects entries using the configured capacity.
ngx_cache_purge_module.c Registers, initializes, inherits, and validates the directive.
docs/​DIRECTIVES.md Documents syntax, limits, and performance/memory trade-offs.

Review effort: Balanced

Comment thread t/purge_runtime.t Resolved
@hsntgm

hsntgm commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

@yaoge123 The ASan and UBSan CI failures were already fixed in #79 (commit 8849a4f), so I think this is not caused by your PR. Once PR #79 is merged or the maintainers fix it separately, re-run the CI for your PR.

@denji I had to apply commit 8849a4f to pass the CI, as PR #81 hits the same wall.

A REFRESH page restarts its scan at the cache root, so a selector that
spans many entries is delivered through several pages and the pending
directories are walked again on every one. The page size was a fixed
compile-time constant, which left large caches with no way to trade
per-request memory for fewer repeated walks.

Add cache_purge_refresh_batch (default 32, range 1..256) and allocate
the page entries from the configured value. The array moves to the end
of the scan structure so the extra capacity is part of the same
allocation.

The runtime coverage now warms three entries with a page size of two and
checks that the first page is bounded and that the cursor returns the
remainder, so a regression to the old fixed cap is visible.
@yaoge123
yaoge123 force-pushed the pr/refresh-dir-prune branch from a2e9778 to f1487bc Compare October 29, 2026 09:06
@yaoge123

Copy link
Copy Markdown
Collaborator Author

Thanks — noted that the ASan/UBSan failure is the Vary reload flake fixed by 8849a4f (#79), not this change. I verified the suite locally under ASan/UBSan with that commit applied and it passes; this diff stays focused and CI can be re-run once #79 lands.

Updated the runtime coverage per the review: it now warms three entries with a page size of two and checks that the first page is bounded by the configured value, that the cursor returns the remainder, and that no entry is reported twice. A regression to the previous fixed cap would now fail.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues were identified for the configurable REFRESH page-size change.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

This branch has not been deployed

No deployments
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.

3 participants