JingsongLi commented on PR #10098: URL: https://github.com/apache/paimon/pull/10098#issuecomment-5832522025
Re-reviewed head `79606fa` for production. The in-place conversion has strong end-to-end value. The new procedure fence, latest-schema rollback guard, schema-ID refresh, and REST protocol restriction address most of the earlier findings. Local JDK 8 conversion/rollback/read suites passed 42/42; the Spark, Flink and E2E CI jobs pass. Both Core CI jobs are red because `S3FileIOTest` could not pull its MinIO Docker image, so those jobs still need a rerun. `git diff --check` passed. **P1 — the public direct catalog action can still enable Data Evolution without a fence.** `FileSystemSchemaManager.checkEnableDataEvolution()` expressly accepts `SchemaChange.enableDataEvolution()` when an empty table has no snapshot; `DataEvolutionEnablerTest#testSchemaChangeAloneIsAcceptedOnTableWithoutSnapshot` pins that path. Unlike `DataEvolutionEnabler.run()`, this direct `catalog.alterTable()` call does not commit the empty fence or repair a writer already in flight. I reproduced the race on this exact head with a temporary test using the existing `PausedWriter` hook: create an empty table; start an old-schema writer and pause it just after `FileStoreCommitImpl.tryCommitOnce()` checks the schema; call `catalog.alterTable(TABLE, SchemaChange.enableDataEvolution(), false)`; release the writer. The alter succeeds, the writer commit succeeds, and the sole live file has `firstRowId == null` even though Data Evolution is enabled. The probe passed and was removed from the revie w checkout. The same gap exists if a direct alter is allowed on a marked assignment snapshot while an old writer is in flight. Please make the generic action unavailable to external callers or ensure every allowed entry point performs the same fence-and-repair protocol before reporting success. Add the direct-alter in-flight-writer regression. The documented rollout must also require older writer binaries to stop, since they cannot enforce the new stale-writer check. This is a production merge blocker; keep the PR open for the fix. -- 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]
