kms: performance findings — eight serialized round trips on every CVM boot
Maintainer thường phản hồi trong vòng 1 ngày
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 5/5
- Thời gian dự kiến
- Hơn một tuần
- Mức phù hợp với người mới
- 32/100
- Loại issue
- Tái cấu trúc
- Độ rõ ràng
- Khá rõ ràng
- Mức độ hoạt động
- Sôi nổi
- Công nghệ
- rust, solidity, typescript
- Lĩnh vực
- api, backend, performance, security
Hướng nghiên cứu
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.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
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.
- Ngôn ngữ chính
- Rust
- Star
- 551
- Fork
- 97
- Merge trung bình
- 1 ngày 8 giờ
- Pull request đã merge (30 ngày)
- 182
Chuẩn bị môi trường
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của Dstack-TEE/dstack
-
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 35/100
Dstack-TEE/dstack#1384 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 30/100
Dstack-TEE/dstack#1301 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 55/100
Dstack-TEE/dstack#1300 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 48/100
Dstack-TEE/dstack#1299 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 48/100
Dstack-TEE/dstack#1298 ·
Maintainer thường phản hồi trong vòng 1 ngày
Tất cả issue của Dstack-TEE/dstack
Issue tương tự
-
area:casework bug criticality:p3 triage:needs-implementation
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 84/100
registrystack/registry-stack#1623 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
DioxusLabs/anyrender#98 ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 76/100
leptos-rs/leptos#4885 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 82/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
longbridge/gpui-kit#3276 ·
Maintainer thường phản hồi trong vòng 1 ngày