bneradt opened a new pull request, #13603:
URL: https://github.com/apache/trafficserver/pull/13603
Unblocks #9636 by decoupling the well-known string (WKS) table from the
on-disk cache format, so the table can be modernized without invalidating
anyone's production cache.
### The problem
A marshalled header stores indexes into the WKS table right next to the
strings those indexes stand for: `MIMEField::m_wks_idx` for every field name,
`HTTPHdrImpl::u.req.m_method_wks_idx`, `URLImpl::m_scheme_wks_idx`, plus the
presence bits and slot accelerators derived from them. Change the table and
every stored index denotes a different string than it did when the object was
written — `GET` reads back as `PUSH`, `http` as `wais`, and `Cache-Control`
becomes invisible to lookups.
Alan Carroll put the backward-compatibility half of the answer in place back
in a62d3a3b19 (PR-1794): `CacheVC::load_http_info()` recomputed the field
indexes when an object's version was older than the running one. That was never
completed, and two later changes broke the plan outright.
### What this changes
**1. Rebuild every WKS-derived value at unmarshal, unconditionally.**
The strings are all present in the object, so the indexes are a pure cache
over them and can always be rebuilt. `HTTPInfo::unmarshal()` now calls
`HTTPHdrImpl::recompute_wks_indices()` on both headers once the heaps are
swizzled. This is the right choke point: the `CacheAltMagic` check keeps it to
once per marshalled buffer, so it lands on the read that already paid for a
disk access or a RAM-cache decompression, and it covers every reader of a
marshalled object.
Doing it here fixes two gaps in the old `load_http_info()` version:
- It never rebuilt the **method** or **URL scheme** indexes. Both
`http_hdr_method_get()` and `URLImpl::get_scheme()` answer from the index in
preference to the stored string, so a stale one silently reports the wrong
method or scheme into alternate selection.
- Its `!f.doc_from_ram_cache` guard ("ram cache is always already fixed up")
is wrong when `proxy.config.cache.ram_cache.compress` is enabled: the RAM cache
then holds the object *still marshalled*, so a hit on it reached the reader
unfixed.
**2. Fix the fragment-offset reader dispatch — the actual reason a version
bump invalidated the cache.**
`unmarshal_helper()` selected `HTTPInfo::unmarshal_v24_1()` with `version <
CACHE_DB_VERSION`. The 24.1 reader rebuilds fragment offset tables the pre-24.2
way, so that comparison is only correct while `CACHE_DB_VERSION == 24.2`.
**Bumping the minor version would have sent every object the previous release
wrote through the wrong reader**, corrupting the fragment table of any object
with more than four fragments. It now compares against
`CACHE_DB_FRAG_OFFSET_TABLE_VERSION`, the fixed version the layout actually
changed at.
**3. Bump `CACHE_DB_MINOR_VERSION` 24.2 → 24.3, and unfreeze the table.**
This does **not** clear anyone's cache: stripe validation looks only at the
major version, and this build reads every object written at an older minor
version. It means an ATS older than 24.3 refuses objects this build writes
rather than resolving their indexes against its own table — which is what makes
future table changes safe, permanently. With that in place the append-only
`_hdrtoken_strs_frozen` ledger from #13559 has nothing left to enforce and is
removed; a `static_assert` that the table still fits `int16_t` replaces it.
### Compatibility
| | |
|---|---|
| Existing 24.2 objects read by this build | Served normally, indexes
rebuilt. **Cache preserved.** |
| 24.3 objects read by an older ATS | Rejected as a future version →
refetch. No misreads. |
| Objects written against any other WKS table, by any 24.3+ build | Served
normally. |
### Testing
- Two Catch2 tests in `test_Hdrs.cc`: one on `recompute_wks_indices()`
directly, one end-to-end through `HTTPInfo::marshal`/`unmarshal` with the
stored indexes rotated. Verified they fail without the fix — `method_get()`
returns `PUSH` instead of `GET`, `scheme_get()` returns `wais` instead of
`http`, and `Cache-Control` cannot be found.
- New autest `cache_wks_table_change`. A `TS_HAS_TESTS`-gated
`ATS_TEST_WKS_IDX_SHIFT` hook rotates every index written into a marshalled
heap and clears the derived bits, standing in for a second WKS table
(impossible to have in-process now that the table is `constexpr`). The replay's
assertions turn on ATS *finding* fields in the cached header rather than on
echoing them back, since a cached response prints from the stored strings and
looks right either way. Verified it fails without the fix: ATS cannot find
`Etag`/`Last-Modified` in the cached response and sends a plain GET where it
owes the origin a conditional one.
- Full `test_proxy_hdrs` suite and 16 cache autests pass.
### Follow-up
The actual string churn from #9636 (dropping NNTP and the dead streaming
schemes, adding modern headers from `header_freq` data) is a separate PR on top
of this one, and needs no further cache work.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
