nevzheng commented on PR #12241: URL: https://github.com/apache/gravitino/pull/12241#issuecomment-5228138594
### Phase 2 — initial thoughts @laserninja — I recommend landing the proposed Phase 1 scope. Then we can address the proposed Phase 2 scope below. **P0 — Caching (blocking):** @laserninja @lasdf1234 @roryqi — we need a strong caching story here: a solid design doc / plan (starting from [#12254](https://github.com/apache/gravitino/issues/12254)), not just enabling the existing cache. Cover at least cache keys, invalidation / staleness, multi-replica behavior, and failure modes. **Shared cache across replicas** is something we should try to address — a detail under this general theme. **Metadata / `plan-task` structure:** 1. I agree with the direction / evolution of the metadata (§8.5) — it’s solid. Document and discuss in Phase 2; don’t forget diagrams for the flow / delta. Fields under discussion: keep `table` / `snapshot-id` / `scan`; add `ranges[]` (`manifest`, `entry-start`, `entry-end`) instead of Phase-1 `offset` / `limit`. 2. Open question: is this **(a)** a versioned encoding / compatibility concern, or **(b)** just an implementation milestone (opaque string; encoding can change without an IRC API break)? **Load amplification:** Discussion / doc (§4, §5.15) revealed key pain points to resolve in Phase 2: 1. **Recompute-per-redeem** — each `/tasks` may re-plan the full snapshot; cost scales with plan size, not batch 2. **Multi-replica concurrent fetch** — Iceberg client fetches plan-tasks in parallel; requests land on different replicas by design 3. **Node-local / cold cache** — today’s cache is per-replica and often off by default, so (1)+(2) become the common production posture Mitigations already on the table: **caching** (P0), **metadata / ranges**, **coalesce** concurrent redemptions on one replica, plus other ops levers. The fact that we’re talking about this many mitigations is itself a signal — we should discuss and design for load amplification thoroughly in Phase 2. **MoR delete attachment under ranges:** 1. **Problem** — once `/tasks` only reads named manifest slices, delete attachment is no longer “free” from the planner. `DeleteFileIndex` is package-private; a subtle mistake returns deleted rows (§8.5.4) 2. **We should address this** — don’t leave MoR correctness implicit when ranges land 3. **Sizing** — may be a **Phase 3** item depending on sizing; discuss further (e.g. CoW-first + MoR re-plan fallback vs full MoR in Phase 2) **Production tuning:** If we follow **good defaults + configurability**, this should stay easy — e.g. `scan-plan-task-batch-size` (default 100). Let’s make sure Phase 1 and Phase 2 keep this in mind: revisit under real load; don’t bury tunables as hidden constants. **Other notes (P2/P3):** 1. **Async `plan/{id}`:** Settled for now (defer; sync Option C / [#11635](https://github.com/apache/gravitino/issues/11635)). I think this could be a strong basis for a **stateful** design where Gravitino maintains plan state. No further comment at the moment — notes only; document for later. 2. **LB affinity:** Finer-grained routing detail (stick one scan to one replica) for P2/P3 — not its own workstream. ### Next steps @laserninja @lasdf1234 @roryqi — please open a Phase-2 follow-up ticket and migrate the concerns / discussion above into it. Don’t overthink Phase 2 right now — **focus on landing Phase 1**. Best, Nevin -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
