suxiaogang223 opened a new pull request, #70257:
URL: https://github.com/apache/doris/pull/70257
### Background
Paimon writers prepare data files independently and then publish a snapshot.
Publication must prevent an existing snapshot version from being overwritten,
even when writers run in different JVMs or engines. Doris currently delegates
to the SDK without checking that protection before starting a write.
### What problem does this PR solve?
Issue Number: None
Related PR: None
PR Type: Bugfix
Problem Summary: Unprotected rename-based snapshot publication can hide a
previously acknowledged write. This PR checks the resolved table's publication
mechanism before creating the BE sink and rejects unsupported mechanisms by
default. The default-off catalog property `paimon.allow-unsafe-writes` lets
operators explicitly retain writes for externally serialized workloads. Most
changed files are existing filesystem write tests receiving that explicit
opt-in; new contract probes cover local cross-JVM publication and JDBC lease
takeover.
### Details
- **Change map:** 133 added lines in production/build configuration, 485 in
tests, and 94 in documentation; no generated result files. Runtime logic stays
inside the Paimon connector.
- **Frequency:** Multiple independent writers can target the same snapshot
version. This includes external engines and standalone compaction jobs; no
production incident frequency is claimed.
- **Root cause:** Local rename protection is JVM-local; object-store rename
need not provide no-overwrite publication. A configured `lock.enabled=true`
does not establish that the SDK actually selected a lock. JDBC lease expiry
also does not fence the previous owner.
- **Why existing tests missed it:** Successful bounded concurrent writes do
not exercise the absent-file race across JVMs or an expired owner resuming
after takeover.
- **Fix:** Admit catalog-managed snapshot version commits with a loader,
standard HadoopFileIO on HDFS, or an enabled Hive catalog lock. Reject
unsupported/unknown mechanisms, including JDBC leases without independent
protection. Check through the existing authenticated and TCCL-pinned connector
context. Log table and reason when an operator opts in.
- **Other paths and branches:** INSERT, INSERT OVERWRITE, UPDATE, DELETE and
MERGE share the gate. No release-branch change, SDK commit rewrite, or S3
conditional-write implementation is included.
- **Behavior change:** Existing unprotected filesystem and JDBC writers need
a supported commit mechanism or explicit `ALTER CATALOG ... SET PROPERTIES
('paimon.allow-unsafe-writes'='true')` for externally serialized workloads. The
opt-in supplies no lock. All engines and compaction jobs must be serialized.
Reads/catalog replay remain available, and no restart is needed for subsequent
planning. Previously planned writes and older FEs during rolling upgrade keep
their previous behavior.
### Testing
Run locally, without compiling Doris:
- `mvn -o -f fe/pom.xml -pl :fe-connector-paimon -am validate -DskipTests
-Dmaven.build.cache.enabled=false` — passed, including Checkstyle.
- `bash build-support/check-fe-connector-imports.sh` — passed.
- `git diff --check` — passed.
**Compilation, FE unit tests, SDK contract probes, and external regression
tests have not run. They are pending validation on Fedora.** An initial
module-only Maven validation could not resolve locally installed reactor POMs;
the offline reactor validation above succeeded.
Added coverage:
- Property defaults, CREATE/ALTER validation, and exclusion from SDK options.
- Actual catalog/lock/filesystem admission, resolving FileIO, unsupported
implementations, toggling the opt-in, and all five DML operations before
transaction binding/sink creation.
- Two JVMs publishing local snapshots with a controlled absence-check
barrier; verifies visibility of the first acknowledged row before and after the
second publication.
- Actual SDK JDBC locks on a test-only SQLite database: explicit expiry,
takeover, old-owner release, and a third acquisition during the new owner's
callback.
- `test_paimon_write_boundary`: default rejection, ineffective lock flag,
opt-in writes, opt-out rejection, unchanged reads/snapshots, reusing existing
output assertions.
- `test_paimon_jdbc_catalog`: default rejection before explicitly bypassing
admission for the existing bounded SDK concurrency stress cases. Those cases do
not qualify JDBC as a supported default mechanism.
### Notes for Reviewers
Start with `PaimonWriteSafety` and the default admission table in
`fe/fe-connector/docs/paimon-write-safety.md`. The accepted Hive-lock and
catalog-version paths still rely on the metastore/server contract and
compatible configuration across all writers. HDFS/HMS/REST and mixed-engine
integration environments are needed to finish validation. This change covers
admission and explicit operator consent; comprehensive lock metrics and
upstream conditional publication/fencing remain follow-up work.
<details>
<summary>Static self-review checkpoints</summary>
| Checkpoint | Conclusion |
| --- | --- |
| Goal and evidence | Early admission plus opt-in implemented; unit, SDK
contract, and regression evidence added but not executed. |
| Scope | Connector-only gate and property; tests updated individually; no
general connector SPI change. |
| Concurrency and lock ordering | No new production threads, locks, or
shared mutable state. SDK coordination is inspected rather than replaced. |
| Lifecycle | No retained handles or background resources. Test processes,
executors, readers, connections and pools have cleanup paths. |
| Configuration | Default false; statement validation; standard catalog
rebuild after ALTER; no FE restart required. |
| Compatibility | Intentional default admission change documented; no
storage format or exported-symbol change; older/in-flight planning remains a
rolling-upgrade limit. |
| Thrift/protobuf | Unchanged. |
| Parallel paths | All five DML operations use planWrite; CTAS/INSERT SELECT
share the insert gate. |
| Conditional checks | Actual SDK environment and resolved FileIO determine
admission; unknown mechanisms fail closed; local/viewfs/JDBC restrictions have
concrete reasons. |
| Test coverage | Positive/negative admission, toggling, malformed
configuration, cross-JVM and lease-expiry probes added; runtime verification
pending. |
| Expected results | No output files added or edited; existing ordered
results should remain unchanged because rejected DML publishes no snapshot. |
| Observability | Rejection includes table/reason/remediation; opt-in emits
WARN. Full lock telemetry is outside this change. |
| Transactions/persistence | No transaction-state or journal change; catalog
property uses existing persistence/replay. No cross-FE recovery claim. |
| Writes/crashes | Rejects before BE file generation; keeps SDK commit
semantics. External serialization remains the operator's responsibility when
opting in. |
| FE/BE propagation | Gate and property remain FE-local; no new BE variable.
|
| Performance | Constant-time metadata inspection; SDK FileIO resolution is
cached; no new remote lock acquisition or snapshot listing during admission. |
| Other limits | Backend integration and fault tests remain required on
Fedora; static checks do not establish runtime correctness. |
</details>
### Release note
Paimon writes now require supported snapshot publication protection by
default. Existing unprotected single-writer deployments can explicitly enable
`paimon.allow-unsafe-writes` after serializing every table writer, including
Spark/Flink and compaction jobs. This property does not add a lock or make
concurrent writes safe. Reading remains available.
### Check List (For Author)
- Test: Static checks passed; compilation and runtime tests pending Fedora.
- Behavior changed: Yes (see Details).
- Does this need documentation: Yes; connector guide included. Website
documentation needs a follow-up before release.
- Pre-submission status: Partially verified: Checkstyle/import/diff checks
only; Fedora compilation and runtime validation pending.
### AI Disclosure
- Tool: Codex, gpt-6.1-sol.
- Generated: Implementation, tests, connector documentation, and PR
description.
- Human review: Pending author review and Fedora validation.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
