Optimize cache miss path with deferred cache write by bryancall · Pull Request #12782 · apache/trafficserver

bryancall · 2026-01-05T22:27:16Z

Add configuration option proxy.config.http.cache.defer_write_on_miss to defer opening the cache for write until after response headers are received and evaluated for cacheability.

Performance Impact

When enabled, this optimization:

Avoids unnecessary cache overhead for non-cacheable responses
Can improve throughput ~21x for non-cacheable content (e.g., Cache-Control: no-store responses)

Trade-offs

When enabled:

May affect read-while-write functionality
May reduce request coalescing for popular uncached URLs
Recommended only for traffic patterns with mostly unique URLs or predominantly non-cacheable content

Configuration

proxy:
  config:
    http:
      cache:
        defer_write_on_miss: 1  # 0=disabled, 1=enabled

How it works

On cache miss, set cache_open_write_deferred=true instead of immediately opening cache for write
Go to origin and fetch response headers
Check if response is cacheable:
- If not cacheable (no-store, etc.): skip cache write entirely
- If cacheable: open cache for write and proceed normally

Note: Default is currently enabled for CI testing. Will be changed to disabled before merge. After CI passed on setting the default to 1, I changed it to be 0.

masaori335

Deferred cache write seems useful if we know almost all the responses are not cacheable but still need to enable http cache for some reason.

Please add docs for this new config.

Copilot

Pull request overview

This PR introduces a configuration option proxy.config.http.cache.defer_write_on_miss to optimize the cache miss path by deferring cache write operations until after response headers are received. This avoids unnecessary cache overhead for non-cacheable responses, potentially improving throughput significantly (~21x for Cache-Control: no-store responses). The default is disabled (0) to maintain safe, backwards-compatible behavior.

Key Changes

New configuration option cache_defer_write_on_miss (default: 0/disabled) for deferring cache writes
New function handle_deferred_cache_write_complete to handle cache write completion after response evaluation
Modified HandleCacheOpenReadMiss to set deferred write flag when configured
Modified handle_no_cache_operation_on_forward_server_response to conditionally open cache for write after response header evaluation

Reviewed changes

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

Show a summary per file

File	Description
`src/proxy/http/HttpTransact.cc`	Core logic for deferred cache write feature including new handler function and integration points
`src/proxy/http/HttpConfig.cc`	Configuration setup and reconfigure logic for new option
`include/ts/apidefs.h.in`	API enum `TS_CONFIG_HTTP_CACHE_DEFER_WRITE_ON_MISS` for plugin access
`include/proxy/http/OverridableConfigDefs.h`	X-macro definition for auto-generating configuration mappings
`include/proxy/http/HttpTransact.h`	State struct member `cache_open_write_deferred` and function declaration
`include/proxy/http/HttpConfig.h`	Configuration struct member `cache_defer_write_on_miss`
`configs/records.yaml.default.in`	Default configuration documentation and value (0/disabled)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bryancall · 2026-02-13T19:18:20Z

[approve ci]

masaori335 · 2026-02-24T00:33:27Z

Seems this has conflicts now.

Add configuration option proxy.config.http.cache.defer_write_on_miss (default: 0/disabled) to defer opening the cache for write until after response headers are received and evaluated for cacheability. When enabled, this optimization: - Avoids unnecessary cache overhead for non-cacheable responses - Can improve throughput ~21x for non-cacheable content Trade-offs when enabled: - May affect read-while-write functionality - May reduce request coalescing for popular uncached URLs - Recommended only for traffic patterns with mostly unique URLs or predominantly non-cacheable content The optimization works by: 1. On cache miss, set cache_open_write_deferred=true instead of immediately opening cache for write 2. Go to origin and fetch response headers 3. Check if response is cacheable: - If not cacheable (no-store, etc.): skip cache write entirely - If cacheable: open cache for write and proceed normally

Add defer_write_on_miss to OVERRIDABLE_CONFIGS to enable per-transaction override via: - conf_remap plugin in remap.config - TSHttpTxnConfigIntSet() plugin API - Lua plugin, header_rewrite, etc.

Add the missing registration for proxy.config.http.cache.defer_write_on_miss in RecordsConfig.cc so the records system recognizes the config variable.

bryancall · 2026-03-16T22:00:39Z

[approve ci autest]

zwoop

Summary of my (and Claude) comments;

How is this tested? Should we add some tests, or rely on existing tests?
No documentation for new configuration
We may want to documented interaction with cache_open_write_fail_action
Similarly, document interaction with read-while-write

zwoop · 2026-03-30T14:51:04Z

 # https://docs.trafficserver.apache.org/en/latest/admin-guide/files/records.yaml.en.html#proxy-config-http-cache-when-to-revalidate
      when_to_revalidate: 0

+# Defer cache open write until response headers are received.


Instead of this long comment, shouldn't it just link to the relevant docs anchor?

zwoop · 2026-03-30T14:52:53Z

+//
+///////////////////////////////////////////////////////////////////////////////
+void
+HttpTransact::handle_deferred_cache_write_complete(State *s)


This one is hard to review :) Are there any existing tests that hits this code (with the setting enabled, obviously)? If not, this feels like we ought to have an autest (ideally start a new proxy-verify "framework" test?).

zwoop · 2026-03-30T14:54:16Z

    s->cache_info.action = CacheAction_t::NO_ACTION;
+  } else if (s->txn_conf->cache_defer_write_on_miss) {
+    // Defer cache open write until after response headers are received.
+    // This avoids cache overhead for non-cacheable responses but may


In general, I find Claud's comments to be overly verbose. If needed, this feels like it can be a one-line. It's pretty clear that

} else if (s->txn_conf->cache_defer_write_on_miss) {

will "defer cache open write" ... If the code is self documenting, we shouldn't state the obvious. I'd suggest you look through all the comments (or ask to) and remove the comments that are obvious.

zwoop · 2026-03-30T14:55:17Z

+    TxnDbg(dbg_ctl_http_trans, "[hncoofsr] deferred cache write, response %s cacheable", cacheable ? "is" : "is not");
+
+    if (cacheable) {
+      // Response is cacheable - open the cache for write now


Again, stating the obvious in this comment, if (cacheable) is self explanatory.

zwoop · 2026-03-30T14:56:08Z

  MgmtByte cache_open_write_fail_action = 0;

+  ////////////////////////////////////////////////
+  // Defer cache open write until response hdrs //


Probably a useful comment, but this is almost too short :) Shouldn't it be something like "...until response headers are received" ?

zwoop · 2026-03-30T14:57:13Z

    bool skip_ip_allow_yaml           = false;

+    /// True if we deferred opening the cache for write until after response headers.
+    /// This is an optimization to avoid cache overhead for non-cacheable responses.


The second comment here sounds like something that would be documented as part of the new setting. We know why we added this ...

zwoop · 2026-03-30T14:59:27Z

  }
 }

+///////////////////////////////////////////////////////////////////////////////


This is a good comment :)

zwoop · 2026-03-30T15:00:27Z

+  }
+
+  // If we get here, cache write failed - continue without caching
+  // The original handle_no_cache_operation_on_forward_server_response will be called


Well, yeah, I can see that handle_no_cache_operation_on_forward_server_response is called ...

zwoop · 2026-03-30T15:01:38Z

  //       #  6 - retry cache read on write lock failure, if retries exhausted serve stale if allowed, otherwise goto origin
  {RECT_CONFIG, "proxy.config.http.cache.open_write_fail_action", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
  ,
+  //       #  defer_write_on_miss:


I don't know that we need this verbose documentation here either, either leave it out, or refer to the Docs section as before.

zwoop · 2026-03-30T20:03:24Z

+    handle_cache_operation_on_forward_server_response(s);
+    return;
+
+  case CacheWriteLock_t::FAIL:


From Claude, sounds like there's a bit of overlap between these two settings, that should at least be documented.

HttpTransact.cc:3364: handle_deferred_cache_write_complete FAIL case does not consult cache_open_write_fail_action. The normal path in handle_cache_write_lock (line 3156) checks this setting and can return 502 via ERROR_ON_MISS, ERROR_ON_MISS_STALE_ON_REVALIDATE, or ERROR_ON_MISS_OR_REVALIDATE. The deferred path unconditionally treats FAIL as LOCK_MISS and serves the origin response. This is arguably correct (we already fetched from origin, so returning 502 would be wasteful), but the interaction between defer_write_on_miss and cache_open_write_fail_action needs to be documented as a known trade-off. Users relying on ERROR_ON_MISS for thundering-herd protection should understand that the deferred path bypasses it.

zwoop · 2026-03-30T20:10:56Z

+    break;
+
+  case CacheWriteLock_t::READ_RETRY:
+    // This shouldn't happen for deferred write since we don't have an object to read


If this shouldn't happen, should it be a InkAssert() at least ? Not sure that TxnDbg is the right choice here either, maybe a notice or something else ?

bryancall added this to the 10.2.0 milestone Jan 5, 2026

bryancall added Cache Performance labels Jan 5, 2026

bryancall self-assigned this Jan 5, 2026

masaori335 self-requested a review January 5, 2026 22:34

bryancall force-pushed the fix-deferred_cache_write branch from 059afe8 to 5ceb59c Compare January 6, 2026 22:21

bryancall marked this pull request as ready for review January 6, 2026 22:27

masaori335 previously approved these changes Jan 8, 2026

View reviewed changes

bryancall requested a review from Copilot January 9, 2026 14:54

Copilot started reviewing on behalf of bryancall January 9, 2026 14:54 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

Comment thread src/proxy/http/HttpTransact.cc

cmcfarlen added this to ATS v10.2.x Feb 10, 2026

bryancall requested a review from masaori335 February 21, 2026 21:05

bryancall force-pushed the fix-deferred_cache_write branch from a373000 to b9a6d7f Compare March 16, 2026 16:39

bryancall added 3 commits March 16, 2026 09:42

Make cache.defer_write_on_miss overridable per-transaction

5d60d2b

Add defer_write_on_miss to OVERRIDABLE_CONFIGS to enable per-transaction override via: - conf_remap plugin in remap.config - TSHttpTxnConfigIntSet() plugin API - Lua plugin, header_rewrite, etc.

Register cache.defer_write_on_miss in RecordsConfig

21d2cd1

Add the missing registration for proxy.config.http.cache.defer_write_on_miss in RecordsConfig.cc so the records system recognizes the config variable.

bryancall force-pushed the fix-deferred_cache_write branch from b9a6d7f to 21d2cd1 Compare March 16, 2026 16:43

zwoop requested review from zwoop March 30, 2026 14:50

zwoop requested changes Mar 30, 2026

View reviewed changes

zwoop reviewed Mar 30, 2026

View reviewed changes

bryancall removed this from ATS v10.2.x Apr 20, 2026

Conversation

bryancall commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance Impact

Trade-offs

Configuration

How it works

Uh oh!

masaori335 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

bryancall commented Feb 13, 2026

Uh oh!

masaori335 commented Feb 24, 2026

Uh oh!

bryancall commented Mar 16, 2026

Uh oh!

zwoop left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bryancall commented Jan 5, 2026 •

edited

Loading