kms: performance findings — eight serialized round trips on every CVM boot
I maintainer di solito rispondono entro 1 giorno
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Idoneità per principianti
- 32/100
- Tipo di issue
- Refactoring
- Chiarezza
- Abbastanza chiara
- Stato di attività
- Attiva
- Stack tecnologico
- rust, solidity, typescript
- Ambito
- api, backend, performance, security
Direzione di ricerca
Treat this as a collection of separate changes rather than one task. Start with main_service.rs:360-369 and auth-eth-bun/index.ts:131-198, then read main_service/upgrade_authority.rs and dstack/http-client/src/lib.rs:90; choose one bounded optimization and trace its existing callers. Done means the selected path removes the stated repeated work without changing authorization or revocation behavior.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
A read of the KMS Rust service, all four auth backends, both contracts, and the verifier/ra-rpc paths the KMS drives, looking for cost rather than correctness. No benchmarks were run — every number below is a complexity class, a counted round trip, or an estimate labelled as such.
Readability and maintainability findings from the same sweep are collected separately; this issue is only the parts that cost time or bandwidth.
Two findings from the sweep turned out to be correctness bugs already fixed in #1247 (the auth-simple empty-device-allowlist fail-open, and the SNP device_id 64-vs-32-byte mismatch), so they are not repeated here.
The headline: ~8 serialized network round trips per GetAppKey
main_service.rs:360-369, auth-eth-bun/index.ts:131-198
get_app_key makes two sequential POSTs to the auth API — ensure_self_allowed() → POST /bootAuth/kms, then ensure_app_boot_allowed() → POST /bootAuth/app. And EthereumBackend.checkBoot makes four sequential chain RPCs each:
const chainId = await this.client.getChainId(); // 1
const head = await this.client.getBlockNumber(); // 2
response = await this.client.readContract({ isAppAllowed, blockNumber }); // 3
gatewayAppId = await this.client.readContract({ gatewayAppId, blockNumber }); // 4
That is 8 strictly serialized Ethereum RPC round trips on every CVM boot, each preceded by a fresh TCP/TLS connect (see below). Against a hosted endpoint at a plausible 80–150 ms RTT that is 0.6–1.2 s of boot latency, all of it avoidable.
Three of the four are unnecessary as written:
getChainId()is invariant for a given endpoint — a startup assertion, not a per-decision one.- The two
readContracts are pinned to the sameblockNumberand are independent:awaitin a loop wherePromise.allis correct. - viem's HTTP transport supports
http(rpcUrl, { batch: true }), which coalesces JSON-RPC calls issued in the same tick into one POST — turning steps 3+4 into a single request.
Fix: assert chainId once at startup, Promise.all the two reads, construct the transport with { batch: true }. 4 sequential round trips → 2. ~15 lines.
Separately, the two is_app_allowed calls in Rust are independent and could run under tokio::try_join! — with the trade-off that the app query would then be issued even when the KMS's own self-authorization is about to fail. Worth stating explicitly if taken; the viem half gets most of the win without it.
A fresh reqwest::Client per authorization decision
main_service/upgrade_authority.rs:98,102
async fn http_get<R: DeserializeOwned>(url: &str) -> Result<R> {
send_request(reqwest::Client::new().get(url), url).await
}
The workspace builds reqwest with features = ["rustls", "hickory-dns", ...], so each construction builds a rustls ClientConfig and a Hickory resolver (reading /etc/resolv.conf), then discards the connection pool — guaranteeing a fresh TCP connect, plus a full TLS handshake if the auth API is HTTPS, per decision. On a boot critical path.
This is #741's lesson, and the project already has the answer: dstack/http-client/src/lib.rs:90 keeps static POOLED/FRESH: OnceLock<reqwest::Client> with a comment arguing exactly this — and upgrade_authority.rs:11 already imports from that crate. The good pattern and the bad one are in the same file's import list. verifier/src/verification.rs:1295 (download_image) has the same shape. ~10 lines.
Together these two are ~25 lines across two files and remove roughly half the serialized network latency from every CVM boot. If only one thing here gets done, do these.
auth-simple re-reads and re-validates its whole config per request
auth-simple/index.ts:77-95, called from :95 and :224
Per bootAuth request: an existsSync, a synchronous readFileSync (Bun is single-threaded — this blocks the event loop for every other in-flight request), a JSON.parse, and a full zod walk of apps — O(n) in registered apps with a fresh object graph each time.
The re-read is not accidental: it is what makes a config edit take effect immediately, the same property repeated_authorization_is_never_served_from_a_decision_cache pins on the Rust side. So the fix must not be a TTL cache. statSync and reuse the parsed value when mtimeMs and size are unchanged: revocation-on-edit is preserved exactly (an edit changes mtime) and the per-request cost drops to one stat. ~20 lines.
And in the same file, a linear scan of something that is already a map (:173-175):
const appConfig = Object.entries(config.apps).find(([id]) => normalizeHex(id) === appId)?.[1];
Object.entries materializes an array and normalizeHex allocates a string per entry, per request. Alongside it, osImages.map(normalizeHex), composeHashes.map(normalizeHex) and devices.map(normalizeHex) rebuild normalized arrays every call and then .includes() them. Normalizing once at load (behind the memo above) into Map + Set makes all of it O(1) with zero per-request allocation. ~25 lines, folds naturally into the memo.
The vm_config blob: 3 copies and 4 JSON parses per GetAppKey
main_service.rs:285-301,324; verifier/src/verification.rs:793-810; dstack-attest/src/attestation.rs:410-427,932-935
self.verify_os_image_hash(vm_config_str.into(), att) // copy 1: &str -> String
// then, inside:
let raw_config = if vm_config.is_empty() { attestation.config.clone() } // copy 2
else { vm_config.clone() }; // copy 3
raw_config is consumed only by the SEV-SNP arm, so on the TDX path — the dominant one — copies 2 and 3 are dead weight.
The parse count is worse: decode_vm_config_with_fallback parses the string twice (once as serde_json::Value, once as VmConfig), and it is invoked twice per request — from decode_app_info_ex and from verify_os_image_hash. Four full JSON parses of the same string per GetAppKey, and for SEV-SNP that string embeds a serialized measurement document, so it is not small.
Fix now: change verify_os_image_hash(vm_config: String) → (&str) and make raw_config a borrow. Kills all three copies, changes no semantics, ~8 lines. The parse-once refactor is bigger and wants its own change.
Concurrent boots of the same image duplicate everything, and can fail each other
verifier/src/verification.rs:385-417,623-641,1280-1377
Neither ensure_image_downloaded nor load_or_compute_measurements coalesces identical in-flight work.
- Duplicated work. N CVMs booting the same new image simultaneously — a fleet restart, the realistic case — all miss
metadata.json, all download the full tarball, all shell out tosha256sum -cover it, all extract it, then all miss the measurement cache and all recompute the sameMachine::measure_with_logs(). O(N) CPU and bandwidth where O(1) is correct. - Blocking the executor.
extract_image_archive,prune_unlisted_image_files,load_measurements_from_cache,store_measurements_in_cacheandcompute_measurementsare all synchronousfs_err+ CPU work called directly fromasync fnwith nospawn_blocking. Extracting a multi-hundred-MB image and SHA-384-ing a kernel + initrd each pin a Rocket worker for seconds;[default] workers = 8inkms.toml, so eight concurrent cold boots stall the whole listener. - A race, not just waste.
download_imagefinishes withremove_dir_all(dst_dir)thenrename(extracted_dir, dst_dir). Two concurrent downloads of the same hash: A renames into place, B then removes A's directory — while a third request is mid-read_to_string(metadata_path)and fails with "Failed to read image metadata". Concurrent boots can fail each other.
This is explicitly not a decision cache: the image is addressed by its own hash and the measurement is a pure function of vm_config_cache_key. Neither is an authorization decision.
The spawn_blocking half stands alone and is small. The coalescing half (a Mutex<HashMap<String, Weak<Shared<…>>>> keyed by hash so the second caller awaits the first) is a judgement call — the number justifying it is "N concurrent cold boots cost N× and can fail each other". A bounded in-memory LRU in front of the file measurement cache would also remove a file read + JSON parse from every warm request.
Smaller
| where | what | cost |
|---|---|---|
main_service.rs:455-457 |
GetMeta re-reads and re-parses bootstrap-info.json from disk on every call — and it is immutable after bootstrap. With client_auth_mandatory() == false, any unauthenticated caller can drive it. (It also swallows a corrupt file through two chained .ok()s, which is a separate problem.) |
load once into KmsStateInner |
main_service.rs:549-555, onboard_service.rs:176-186 |
Two whole-attestation clones to read one field. into_v1() takes self, so cloning is the path of least resistance — but the thing copied is quote + cert chain + event log, bounded by MAX_ATTESTATION_BYTES = 10 MiB. onboard_service.rs:176 clones and converts the whole attestation purely to match on platform's discriminant, then converts again. Typical cost is small; the bounded worst case is a 10 MiB memcpy per SignCert from an untrusted caller. |
a borrowing variant(&self) accessor |
main_service.rs:127-134 |
ClearImageCache calls fs::remove_dir_all on an async handler; an image cache can be many GB. Admin-only and rare, but spawn_blocking is the right shape. |
S |
IAppAuth.sol:36-46 |
advisoryIds (string[]) is ABI-encoded into every eth_call by both Ethereum backends and read by no contract. An unread string[] is the most expensive member of the struct to encode and send, on a call made 8 times per boot. (Whether to wire it into policy or drop it is a contract-deployment decision, tracked separately.) |
— |
DstackKms.sol:107-109, DstackApp.sol:142-144 |
_emitPolicy does keccak256(bytes(policy)) where policy is always a string literal at the call site. Hoisting to bytes32 constant saves the copy and the hash. Being honest: ~100 gas on owner-only admin writes. Listed for completeness, not because it matters; it should only ride along with another contract change. |
negligible |
Checked and found clean
So a later reader knows where not to look again: kms/src/crypto.rs (the cleanest file in the component — a verbatim pre-refactor reference implementation plus golden vectors in the test module, so the compatibility claim is machine-checked every run); admin_auth.rs and admin_service.rs; config.rs; send_request's bounded error context; onboard_service.rs's validate_onboarding_domain and ca_cert_expires_within; ra-rpc/src/ratls_client_verifier.rs (the expensive half of verification is deliberately kept out of the handshake and documented as such, so unauthenticated peers cannot drive quote verification from a TLS hello); cert-client/src/lib.rs; the gas shape of both contracts (no storage reads in loops, no unbounded iteration, calldata used correctly, __gap present in both — and the PolicyChanged audit-event design is a genuinely good addition, it makes the whole policy reconstructable from logs); the Manage.s.sol/Query.s.sol loops (one transaction per item is unavoidable given the contract's one-value-per-call setters); and the key derivation in get_app_key itself.
- Lingua principale
- Rust
- Stelle
- 555
- Fork
- 97
- Merge medio
- 1g 3h
- PR unite (30g)
- 195
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di Dstack-TEE/dstack
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
Dstack-TEE/dstack#1384 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 30/100
Dstack-TEE/dstack#1301 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 3/5 1-2 giorni Idoneità per principianti 55/100
Dstack-TEE/dstack#1300 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 48/100
Dstack-TEE/dstack#1299 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 48/100
Dstack-TEE/dstack#1298 ·
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di Dstack-TEE/dstack
Issue simili
-
enhancement
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
zcashlabs/thus-spoke-zakura#153 ·
I maintainer di solito rispondono entro 1 giorno
-
app bug windows-os
Difficoltà 2/5 1-3 ore Idoneità per principianti 67/100
I maintainer di solito rispondono entro 1 giorno
-
Improve sublime text syntaxApertaeditor good first issue
Difficoltà 2/5 1-3 ore Idoneità per principianti 65/100
funnyboy-roks/inq#54 ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
pnpm/pnpm#16635 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
[Bug]: Bedrock request metadata forwarding does not work for /embeddingsForse già presa Una pull request collegata a questa issue è aperta o già unita. Apertabug llm translation
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
I maintainer di solito rispondono entro 1 giorno