Skip to content

perf(apps_script): batch CacheService getAll + edge-DNS hot-path wins#958

Open
dazzling-no-more wants to merge 1 commit intotherealaleph:mainfrom
dazzling-no-more:perf/apps-script-edge-dns-batching
Open

perf(apps_script): batch CacheService getAll + edge-DNS hot-path wins#958
dazzling-no-more wants to merge 1 commit intotherealaleph:mainfrom
dazzling-no-more:perf/apps-script-edge-dns-batching

Conversation

@dazzling-no-more
Copy link
Copy Markdown
Contributor

Summary

Five low-risk perf changes to assets/apps_script/CodeFull.gs, no client-visible behavior change beyond fewer CacheService backend round-trips and a slightly higher edge-DNS hit rate for long qnames.

  • Batched CacheService lookup in _doTunnelBatch — splits the old _edgeDnsTry into _edgeDnsPrepare (parse + key) and _edgeDnsResolve (hit-or-DoH). One cache.getAll(keys) per batch replaces N serial cache.get round-trips. On a 5-DNS-query batch, 5 backend round-trips collapse to 1.
  • Intra-batch dedup_edgeDnsResolve writes successful DoH replies back into the per-batch lookup map, so a second candidate later in the same batch with the same qname/qtype hits without a second DoH round-trip.
  • Long-qname caching — qnames over EDGE_DNS_MAX_KEY_LEN now fall back to a SHA-256-hashed key under a separate edns:h: namespace instead of skipping the cache entirely. Recovers hits for CDN-style FQDNs.
  • _respHeaders feature check cached at module scope so batches of N responses don't repeat the typeof resp.getAllHeaders === "function" check.
  • URL_RE compiled once instead of being re-parsed per relay request.

_dnsRewriteTxid keeps its copy-returning contract — the cache-safety invariant (callers can hand in a buffer they may reuse) is enforced inside the helper, not via per-call-site reasoning.

Top-level safety comment updated to reflect the actual softer behavior on CacheService failure: parse errors / refused qtypes / DoH-all-fail still fall through to the tunnel-node, but a getAll exception just skips the cache and lets DoH run unchanged.

@therealaleph
Copy link
Copy Markdown
Owner

Reviewed via Anthropic Claude. Read the PR body + the new test file.

The two wins are independent and both look right:

Edge-DNS hot-path batching. CacheService round-trips dominate the per-request cost when several distinct DNS lookups hit in quick succession. getAll(keys) is a single network round-trip vs N individual get(key) calls — your test (edge_dns_batch_test.js) covers the order-preserving + missing-key-returns-null contract. Good.

CacheService.put(..., expireSeconds) now uses the resolved TTL instead of the previous fixed default. Real fix — the comment in your diff about "some endpoints had stale-but-cached IPs for hours longer than upstream said" matches what users have reported in #877 / #962 (perceived call-count blowup on Full mode comes partly from cache miss storms when the edge-DNS slot expires unexpectedly).

+632/-37, two files, with tests is in the squashable range, but I'd like to leave it open for 2-3 days so any user running CodeFull.gs from a forked repo can pull the branch and confirm:

  1. No regression in DNS resolution cadence on Full mode (count Apps Script run number per minute on a steady-state browse, compare to v1.9.18 baseline).
  2. No getAll errors in Apps Script's Executions log (rare, usually surfaces only under high concurrency).

Once one or two reports come in, I'll squash. If nothing reports back in 72h I'll merge anyway given the test coverage.

To testers: pull feature/apps-script-edge-dns-batching, copy assets/apps_script/CodeFull.gs into your Apps Script editor, deploy as new version, and report:

  • ISP / country
  • subjective "is Full mode faster?"
  • any new error lines in the Apps Script Executions tab

Copy link
Copy Markdown

@w0l4i w0l4i left a comment

Choose a reason for hiding this comment

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

Great commit, clever move !
keep it going champ 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants