prefetch: move cache key acquisition to CACHE_LOOKUP_COMPLETE

[mlibbey]

The prefetch plugin read TSHttpTxnCacheLookupUrlGet at POST_REMAP, but the core does not initialize the cache lookup URL until HttpTransact::HandleRequest runs after POST_REMAP returns. The call therefore failed unless a plugin like cachekey was called earlier and populated the URL via TSHttpTxnCacheLookupUrlSet, making prefetch silently dependent on cachekey's presence and remap ordering. Acquire the cache key at CACHE_LOOKUP_COMPLETE instead, which is the first hook where the lookup URL is guaranteed to be set.

On transactions where ATS skips cache lookup (non-cacheable methods, internal redirects, cache disabled), the front-end first-pass admission acquire no longer fires. TXN_CLOSE release is gated on _fetchable, so acquire/release symmetry is preserved.

The four existing prefetch autests all pair cachekey.so ahead of prefetch.so and would have passed with or without this fix. Adds prefetch_no_cachekey.test.py to regression-cover the standalone case.

[mlibbey]

Don't think the autest fail was because of this change -- adding 1 test should not make 114 fail/timeout.

[approve ci autest 2of4]

[mlibbey]

[approve ci autest 2of4]

[Copilot]

Pull request overview

This PR fixes an ordering bug in the prefetch remap plugin by moving cache key acquisition from POST_REMAP to CACHE_LOOKUP_COMPLETE, where the core guarantees the cache lookup URL has been initialized. It also adds a regression gold test to ensure prefetch.so works correctly without cachekey.so being loaded ahead of it.

Changes:

• Move TSHttpTxnCacheLookupUrlGet-based cache key acquisition to TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE and remove POST_REMAP hook usage in prefetch plugin.
• Adjust request-MLoc handling logic to match the new hook set (no longer expecting POST_REMAP).
• Add a new gold test (prefetch_no_cachekey.test.py) covering the standalone prefetch.so configuration.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
plugins/prefetch/plugin.cc	Moves cache key acquisition to CACHE_LOOKUP_COMPLETE and removes POST_REMAP hook registration/handling.
tests/gold_tests/pluginTest/prefetch/prefetch_no_cachekey.test.py	Adds regression coverage for running prefetch.so without cachekey.so.

Comments suppressed due to low confidence (1)

plugins/prefetch/plugin.cc:549

• In the front-end second-pass path, if acquire() succeeds but uniqueAcquire() fails, _fetchable becomes false and TXN_CLOSE will skip releasing the (already-acquired) policy state, causing a leak / incorrect policy accounting. Mirror the schedule() logic: if uniqueAcquire() fails, release() the earlier acquire() before continuing.

        /* second-pass */
        data->_fetchable = state->acquire(data->_cachekey);
        data->_fetchable = data->_fetchable && state->uniqueAcquire(data->_cachekey);
        PrefetchDebug("request is %s fetchable", data->_fetchable ? " " : " not ");

[zwoop]

I think this is just a matter of adding a

 TSHandleMLocRelease(reqBuffer, TS_NULL_MLOC, reqHdrLoc);

when it fails to appendCacheKey().

[mlibbey]

indeed -- thanks! Addressed.

[zwoop]

As commented, this actually fixes an interesting issue (accidentally "prefetching" of likely uncacheable objects). As such, I asked Claude to document this, so here's the suggestion:

  --- a/doc/admin-guide/plugins/prefetch.en.rst
  +++ b/doc/admin-guide/plugins/prefetch.en.rst
  @@ -33,6 +33,15 @@ Description
   On every **incoming** URL request, the plugin can decide to pre-fetch the
   **next object** or more objects based on the common URL path pattern and a
   pre-defined pre-fetch policy.

  +.. note::
  +
  +   The plugin keys its fetch policy off the request's **cache key**, which is
  +   only available once ATS has resolved the cache lookup URL. It therefore acts
  +   at the ``CACHE_LOOKUP_COMPLETE`` hook. On transactions where ATS skips the
  +   cache lookup entirely -- non-cacheable methods, some internal-redirect legs,
  +   or caching disabled (``proxy.config.http.cache.http: 0`` or per remap) --
  +   the plugin never triggers a prefetch. Prefetching into a cache that will not
  +   be consulted is a no-op, so this is expected.
  +
   Currently, most HLS video urls follow a predictable pattern, with most URLs
   containing a segment number. Since the segments are ~10s of content, the normal
   usage pattern is to fetch the incremental segment every few seconds. The CDN

[mlibbey]

As commented, this actually fixes an interesting issue (accidentally "prefetching" of likely uncacheable objects). As such, I asked Claude to document this, so here's the suggestion:

  --- a/doc/admin-guide/plugins/prefetch.en.rst
  +++ b/doc/admin-guide/plugins/prefetch.en.rst
  @@ -33,6 +33,15 @@ Description
   On every **incoming** URL request, the plugin can decide to pre-fetch the
   **next object** or more objects based on the common URL path pattern and a
   pre-defined pre-fetch policy.

  +.. note::
  +
  +   The plugin keys its fetch policy off the request's **cache key**, which is
  +   only available once ATS has resolved the cache lookup URL. It therefore acts
  +   at the ``CACHE_LOOKUP_COMPLETE`` hook. On transactions where ATS skips the
  +   cache lookup entirely -- non-cacheable methods, some internal-redirect legs,
  +   or caching disabled (``proxy.config.http.cache.http: 0`` or per remap) --
  +   the plugin never triggers a prefetch. Prefetching into a cache that will not
  +   be consulted is a no-op, so this is expected.
  +
   Currently, most HLS video urls follow a predictable pattern, with most URLs
   containing a segment number. Since the segments are ~10s of content, the normal
   usage pattern is to fetch the incremental segment every few seconds. The CDN

Thanks! Added a similar section in now.