feat(http): expose the proxy cache a request went through - #321
Open
u5surf wants to merge 2 commits into
Open
Conversation
Add a typed `CacheStatus` mirror of nginx's `NGX_HTTP_CACHE_*`
constants and two safe `Request` accessors:
- `Request::cache_status() -> Option<CacheStatus>` reads
`r->upstream->cache_status` (the same value
`$upstream_cache_status` exposes) and converts the raw
`ngx_uint_t` into a typed variant; the "no cache lookup" sentinel
(`0`) and any unknown value surface as `None`.
- `Request::cache_zone_name() -> Option<&NgxStr>` walks
`r->cache->file_cache->shm_zone->shm.name` to return the
`proxy_cache_path` keys-zone name — handy as the `zone=` label
for cache metrics.
Both are gated by `#[cfg(ngx_feature = "http_cache")]` so builds
without `--without-http_cache` still link.
Unit tests use the `MaybeUninit::zeroed` pattern from nginx#272 to
construct the request / upstream / cache chain on the stack and
verify the null / zero-sentinel / populated paths.
cache_zone_name() gives a metric its zone= label; the numbers to put against it are still out of reach. A module reporting on the cache wants how large it may grow and how much of that it is using. Adds cache_zone_max_size() and cache_zone_used_size(), both in bytes, and cache_zone_block_size() for callers that want the raw unit. Bytes rather than what the struct holds, because what it holds is surprising. ngx_http_file_cache_init divides the configured max_size by the filesystem block size so it can be compared against sh->size, which the cache manager also accumulates in blocks. Reading either field directly answers in blocks, which on a 4k filesystem is a figure some thousands of times too small, and nothing about the field says so. The test covers exactly that: a 1 MiB zone holding 256 KiB reads as 256 and 64 off the struct. Signed-off-by: Y.Horie <u5.horie@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Two accessors for the proxy cache a request went through, and three for the numbers behind it.
CacheStatusis an enum over the eightNGX_HTTP_CACHE_*values, withfrom_rawreturningNonefor the zero that nginx leaves on a request that consulted no cache — so a caller cannot mistake "no lookup happened" for a status.Everything is behind
#[cfg(ngx_feature = "http_cache")], since none of the fields exist otherwise. Built and tested both ways.Why the sizes are worth an accessor
This is the part I would not have got right by reading the struct.
nginx does not keep
max_sizeandsh->sizein bytes.ngx_http_file_cache_initdivides the configured size by the filesystem block size so the two can be compared directly:and the cache manager accumulates
sh->sizein the same unit. Reading either field off the struct answers in blocks — on a 4k filesystem, a figure roughly four thousand times too small, with nothing about the field to suggest it. A metric built that way looks plausible and is wrong.cache_zone_max_size()andcache_zone_used_size()multiply bybsizeand answer in bytes, which is what the configuration said.cache_zone_block_size()is there for anyone who wants the raw unit.The test states the trap directly: a 1 MiB zone holding 256 KiB reads as
256and64off the struct, and as1048576and262144through the accessors.Where this came from
Writing a traffic-status module. These are the four things it reads to report
nginx_vts_cache_requests_totalandnginx_vts_cache_size_bytes, and none of them had an accessor, so that part of the module is still C. Its notes on what keeps it there are here.Testing
cargo test,cargo clippy --all-targets,cargo fmt --checkandcargo docclean, with--features vendoredand again withNGX_CONFIGURE_ARGS=--without-http-cacheso the gated code is compiled out.Seven unit tests over the cache accessors, covering each null in the
r->cache -> file_cache -> shm_zonechain, the zero sentinel, and the block-to-byte conversion.Checklist