Fixes #39208 - Cache global registration script in smart-proxy

[pablomh]

Problem

GET /register returns the same shell script for all hosts sharing the same registration parameters (org, location, hostgroup, activation keys). Under bulk registration — 100+ hosts hitting the same capsule simultaneously — this endpoint is called once per host, each time proxying to Foreman and waiting for the ERB template to render (~103ms in profiling). With WEBrick's MaxClients=100, all threads can be held waiting for identical Foreman responses.

Solution

Cache the rendered script in memory (5-minute TTL) keyed on a canonical form of the request query string (parameters sorted alphabetically, so requests differing only in parameter order share the same cache entry).

Per-key double-checked locking prevents the thundering herd on a cold cache while allowing concurrent requests for genuinely different keys (e.g. different activation keys) to fetch from Foreman in parallel without blocking each other:

• Fast path: unsynchronised Concurrent::Map read, no lock acquired
• Slow path: key-specific Mutex serialises only competing threads; re-checks under lock before fetching from Foreman

The per-key Mutex is evicted from KEY_MUTEXES immediately after the script is written to cache, so KEY_MUTEXES is empty under steady state and bounded to in-flight fetches only.

Only 2xx responses are cached — errors never poison the cache. SCRIPT_CACHE uses Concurrent::Map (already a smart-proxy dependency) for lock-free reads that are safe on all Ruby VMs.

Test plan

• Cache hit: Foreman called once for repeated identical requests
• Per-key isolation: different parameter sets cached independently
• Parameter order independence: requests differing only in param order share the same cache entry
• TTL expiry: expired entries are not served; Foreman is re-called
• Mutex eviction: KEY_MUTEXES is empty after a successful cache write
• Error non-caching: Foreman is called on every request when it errors

Fixes #39208

[ekohl]

In Ruby you can pass blocks and that creates a nice abstraction mechanism. In somewhat pseudo-code:

CACHE = {}
MUTEX = Mutex.new

def cache(key, &block)
  if (value = CACHE[key])
    value
  else
    MUTEX.synchronize do
      CACHE[key] = block.call
    end
  end
end 

def get_value(query)
  cache(query) do
    real_call_here(query)
  end
end

You may need to do something like raising an exception to handle errors from Foreman code (in handle_response) so it doesn't get cached.

[pablomh]

Thanks — we followed exactly this pattern. The cache(key, &block) method uses double-checked locking with per-key mutexes via Concurrent::Map, and errors raise out of the block so they never poison the cache:

def cache(key, &block)
  value = read_registration_cache(key)
  return value if value

  self.class.key_mutex(key).synchronize do
    value = read_registration_cache(key)
    return value if value

    result = yield
    self.class.registration_script_cache[key] = { body: result, at: Time.now }
    self.class.evict_key_mutex(key)
    result
  end
end

The caller:

cache(cache_key) do
  response = Proxy::Registration::ProxyRequest.new.global_register(request)
  raise ScriptFetchError, response unless response.code == '200'
  response.body
end

Non-200 responses raise ScriptFetchError which is caught outside the cache block and passed to handle_response. Covered by tests including test_global_register_does_not_cache_errors.

[pablomh]

I pushed a follow-up refinement on top of this branch.

Main changes:

• cache key now includes auth context, not just normalized query params
• TTL now uses monotonic time instead of Time.now
• per-key mutexes are retained instead of evicted after successful writes
• expired cache entries are removed on read
• added tests for auth-aware cache partitioning and stale-entry cleanup

The main motivation was that two requests with the same query string but different auth can produce different rendered scripts, since Foreman embeds a user-derived token in the registration script. With a query-only cache key, those could incorrectly share a cache entry.

For the auth partitioning, I currently used the raw Authorization header for exact separation and zero hash cost. I left the SHA-256 alternative commented right below the helper so we can choose which tradeoff we prefer in review:

• raw header: exact and cheap
• SHA-256: avoids raw credentials in cache keys

Happy to switch that part if there’s a preference.

[pablomh]

Ping?

[pablomh]

Added identity-safe mutex eviction: after the cache entry is written, the per-key mutex is removed from KEY_MUTEXES since the cache entry itself prevents redundant fetches until TTL expiry. The ensure block guarantees cleanup even if the fetch raises, and equal? prevents accidentally deleting a replacement mutex created by a concurrent miss for the same key.

Without this, KEY_MUTEXES grows with every unique cache key and never shrinks. Practically bounded for typical deployments (few activation key combinations), but prevents unbounded growth if auth-aware cache keys produce high cardinality.

[jeremylenz]

I don't know if this is a pattern that's followed elsewhere, but could this also be proxied cached via apache rather than the application?

[pablomh]

I don't know if this is a pattern that's followed elsewhere, but could this also be proxied via apache rather than the application?

Proxied or cached?

[jeremylenz]

oh I meant cached, yeah

[pablomh]

I haven't seen it done at the proxy level yet.

[pablomh]

Friendly ping when you have a moment: I addressed the earlier feedback and the branch is ready for another look. I'd really appreciate a review when convenient. Thanks!

[pablomh]

Regarding your question and an "extended" answer: I know we can do it at the Apache level, but we haven't done it neither in smart-proxy nor in the main foreman server, so I'm hesitant to introduce a change in the architecture at that level.

[jeremylenz]

Regarding your question and an "extended" answer: I know we can do it at the Apache level, but we haven't done it neither in smart-proxy nor in the main foreman server, so I'm hesitant to introduce a change in the architecture at that level.

@ehelms I would like to get your thoughts on this (caching the global reg script, and where that caching should live.)

[pablomh]

...and when I find content for which we need to have authentication, that usually means that we'd need to replicate all the authentication at the Apache level, so in those cases I prefer to add the caching at the layer that manages the content.

[ehelms]

GET /register returns the same shell script for all hosts sharing the same registration parameters (org, location, hostgroup, activation keys).

What happens if the registration contains other configuration parameters beyond those 4?

[pablomh]

Good catch, that sentence is imprecise. The cache key is not limited to those 4 fields; the implementation uses the full normalized query string, so any additional registration configuration passed as query params gets its own cache entry as well.

On top of that, the cache is also partitioned by auth context, since the rendered script can differ there too. Those 4 params were just the motivating examples from the bulk-registration case, not the full key definition.

I can update the PR description to make that clearer.

[ehelms]

I think this is a good performance improvement for those scenarios mentioned, and that the 5 minute TTL feels right.

[ehelms]

Design wise from me, I gave the code a cursory review.