Skip to content

Conversation

@bryancall
Copy link
Contributor

@bryancall bryancall commented Jan 5, 2026

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

  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

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.

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
@bryancall bryancall added this to the 10.2.0 milestone Jan 5, 2026
@bryancall bryancall self-assigned this Jan 5, 2026
@masaori335 masaori335 self-requested a review January 5, 2026 22:34
@bryancall bryancall force-pushed the fix-deferred_cache_write branch from 059afe8 to 5ceb59c Compare January 6, 2026 22:21
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.
@bryancall bryancall marked this pull request as ready for review January 6, 2026 22:27
masaori335
masaori335 previously approved these changes Jan 8, 2026
Copy link
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

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

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.

@masaori335 masaori335 dismissed their stale review January 8, 2026 02:19

Please add docs for this new config.

@bryancall bryancall requested a review from Copilot January 9, 2026 14:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

// If we get here, cache write failed - continue without caching
// The original handle_no_cache_operation_on_forward_server_response will be called
// but we need to skip the deferred cache write check since we just tried it
// s->cache_open_write_deferred is already false from the first call
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

This comment is misleading. The flag cache_open_write_deferred was set to false at line 4751 when the cache write was initiated, not "from the first call". Consider revising to: "s->cache_open_write_deferred was already set to false before initiating the cache write" for clarity.

Suggested change
// s->cache_open_write_deferred is already false from the first call
// s->cache_open_write_deferred was already set to false before initiating the cache write

Copilot uses AI. Check for mistakes.
Add the missing registration for proxy.config.http.cache.defer_write_on_miss
in RecordsConfig.cc so the records system recognizes the config variable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants