ealeonraz commented on PR #13106: URL: https://github.com/apache/gravitino/pull/13106#issuecomment-5861361562
Thanks, both were real. Pushed a fix and merged main into the branch (it had gone conflicting). **Cold-cache clear race.** You are right that walking the existing `IDX` keys cannot fence a metalake that has nothing indexed yet. `clear()` no longer discovers metalakes from the keyspace at all. A read miss now `SADD`s its metalake into a per-namespace registry set (`<ns>:metalakes`) before it returns, so the registration happens before the caller starts its load; `clear()` and `size()` walk that set and run the per-metalake script for every member, index or not. The ordering argument: if the miss registered before `clear()` read the set, the metalake fence moves and the fill is rejected; if it registered after, the load began after the clear began, so what it fills is not older than the clear. A registration failure drops the miss's fence record, so the fill fails closed. This also removed the whole `SCAN` / primaries-only path, since nothing needs to enumerate keys any more. Tests: `testClearOnAColdCacheRejectsAnInFlightFillOfAnUnindexedMetalake` in the shared base (runs agai nst standalone and cluster), plus unit tests that a miss registers and that a failed registration fails the fill closed. **Cluster IT readiness.** The probe now waits until every expected node reports `role:slave` via `INFO replication`, not just until the slot map lists them. Verified locally against a three-primary, three-replica cluster (30/30). Two more things the merge with main surfaced, both in this push: - `IndexImpl` now snapshots its properties as `Collections.unmodifiableMap(...)`. Kryo's field serializer cannot set that wrapper's delegate under JDK 17 (java.base does not open it), so the serializer now rebuilds the JDK unmodifiable map and collection wrappers from their contents, the same way it already handles the Guava immutables. The existing table round-trip test caught it. - The Flink IT failure (`KryoException: Invalid ordinal for enum org.apache.iceberg.FileFormat`) was this PR's fault: Flink bundles Kryo 2 as `com.esotericsoftware.kryo:kryo`, `:core` now brings `com.esotericsoftware:kryo:5.6.2` with the same package names onto `flink-common`'s test classpath, and Kryo 5 won. Excluded that group from `flink-common`'s `testRuntimeClasspath`; it is the only module that runs Flink jobs with `:core` on the classpath. Local run on the merged head: 65 unit tests, standalone IT 30/30, cluster IT 30/30 with replicas, Lua script tests 5/5, two-store latch suite 4/4, full `:core:test` green. One note on CI: every workflow on the new head shows `startup_failure`, and the other PRs pushed today show the same on all of theirs, so that looks like a workflow-file issue on main rather than something in this branch. Happy to re-push to retrigger once main is green. -- 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]
