SEPURI-SAI-KRISHNA opened a new pull request, #11925:
URL: https://github.com/apache/seatunnel/pull/11925
### Purpose of this pull request
Closes #11923.
`docs/{en,zh}/architecture/features/multi-table.md` presented Java listings
that read as a description of `MultiTableSinkWriter`, but named an API that
does not exist. A reader following the page would write code against methods
and fields that cannot be found in the repository.
Per the direction given by @DanielLeens in the issue thread
([comment](https://github.com/apache/seatunnel/issues/11923#issuecomment-5369664827))
— option 1, mirror the real writer, mark deliberate simplifications as
simplified, keep the narrower `Math.abs` correctness fix in #11721, and update
en and zh together — this PR realigns the documentation. It is docs-only: no
Java file is touched.
### What was wrong
Verified against
`seatunnel-api/src/main/java/org/apache/seatunnel/api/sink/multitablesink/MultiTableSinkWriter.java`:
| Documented | Reality |
| --- | --- |
| `int selectReplica(TablePath, SeaTunnelRow)` | no such method — 0 hits
repo-wide |
| `Optional<Object> extractPrimaryKeyIfPresent(row)` | no such method — 0
hits repo-wide |
| `private final int replicaNum` on the writer | not a field on the writer;
fan-out width is `blockingQueues.size()` |
| `writers.get(identifier); writer.write(element)` | rows are queued via
`offerRowElement(index, element)` and drained by `MultiTableWriterRunnable`
workers |
| `TablePath tablePath = row.getTablePath()` | routing keys off the
`tableId` string via `sinkPrimaryKeys.get(tableId)` |
| random strategy `(int) (System.nanoTime() % replicaNum)` |
`random.nextInt(blockingQueues.size())` |
| two branches (pk / no pk) | three — the unknown-table branch honours
`MultiTableFailurePolicy` |
| `handleSchemaChange(...)` looping replicas (§6.2) |
`applySchemaChange(...)`, which enqueues a `SchemaChangeBarrier` onto every
queue |
`System.nanoTime() % replicaNum` was additionally unsound as illustrative
code: `nanoTime()` may be negative, so it can itself produce a negative index —
the same bug class #11721 fixes one line above it. That line is now removed and
the reason is stated, rather than silently dropped.
### What changed
**`docs/en/.../multi-table.md`**
- **§4.2 Writer** — listing rewritten against the real class: real
signature, real fields (`sinkWriters`, `sinkPrimaryKeys`, `blockingQueues`,
`sinkWritersWithIndex`, `runnable`, `failurePolicy`), the real three-branch
`write(SeaTunnelRow)`, and the queue hand-off. Adds a sentence explaining that
the checkpoint methods must drain the queues first via `checkQueueRemain()`.
- **§5.3 Replica Selection** — both strategies corrected to the real
expressions, each with the reason the naive form is wrong.
- **§6.2 Schema Evolution Routing** — rewritten to the real
`applySchemaChange` → `enqueueSchemaChangeBarrier` path, including the early
return for unrelated tables.
**`docs/zh/.../multi-table.md`** — the zh page has no Java listings (it is
prose plus one formula), so its drift was narrower but real: it described rows
as routed directly to the underlying writer, omitted the failure-policy branch,
and omitted the schema-change barrier. §4.2, §5.3, and §6.2 are aligned in
prose, and the §5.3 formula becomes `(hash(pk) & Integer.MAX_VALUE) mod
replicaNum` with both traps noted. This change is deliberately smaller than the
en one because there was less that was wrong, not because zh was skipped.
Both pages now explicitly state that the listings are simplified and are not
copies of the source, so the remaining gap is honest rather than misleading.
### Relationship to #11721
#11721 fixes the `Math.abs` routing bug in `MultiTableSinkWriter` and
corrects the two `Math.abs` occurrences on this page. It is approved and
awaiting a write-access re-approval.
**The two PRs touch the same regions of this file and whichever merges
second will need a trivial rebase.** I am happy to do that rebase either way.
Note also that the routing line in this PR is written in the post-#11721 masked
form (`hashCode() & Integer.MAX_VALUE`), because documenting the current
`Math.abs` line as the reference pattern would be documenting a known bug. If a
maintainer would rather this PR wait until #11721 merges, that is fine by me —
say so and I will hold it.
### Does this PR introduce a user-facing change?
Documentation only. No config option, default value, public API, or
behaviour is changed.
### How was this patch tested?
Documentation-only change; no code paths are affected. Verification was by
cross-checking every symbol the new text names against the source: all of
`sinkWriters`, `sinkPrimaryKeys`, `blockingQueues`, `sinkWritersWithIndex`,
`runnable`, `failurePolicy`, `offerRowElement`, `offerQueueElement`,
`ensureQueueWorkersSubmitted`, `subSinkErrorCheck`, `handleTableFailure`,
`checkQueueRemain`, `applySchemaChange`, `hasSourceMatchedWriter`,
`enqueueSchemaChangeBarrier`, `dispatchSchemaChangeToTargets`,
`SchemaChangeBarrier`, `MultiTableFailurePhase.RUNTIME_WRITE`, and
`MultiTableWriterRunnable.schemaChangeRequest` are present, and none of
`selectReplica`, `extractPrimaryKeyIfPresent`, `writers.get(identifier)`,
`handleSchemaChange`, or `System.nanoTime()` remains as a recommended pattern
in either page.
### Source paths mirrored
As requested in the issue, so reviewers can check the mapping quickly:
-
`seatunnel-api/src/main/java/org/apache/seatunnel/api/sink/multitablesink/MultiTableSinkWriter.java`
— `write(SeaTunnelRow)`, `applySchemaChange(SchemaChangeEvent)`,
`enqueueSchemaChangeBarrier(SchemaChangeEvent)`, field declarations
-
`seatunnel-api/src/main/java/org/apache/seatunnel/api/sink/multitablesink/MultiTableWriterRunnable.java`
— `QueueElement`, `schemaChangeRequest(...)`
-
`seatunnel-api/src/main/java/org/apache/seatunnel/api/sink/multitablesink/MultiTableSink.java`
— `replicaNum` (§4.1, which was already accurate and is unchanged)
### Check list
* [x] Code changed are covered with tests, or it does not need tests for
this PR — docs-only
* [x] If any new Jar binary package adding in your PR, please add License
Notice according [New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md)
— n/a
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs — this PR *is* the
documentation update
* [x] If you are contributing the connector code, please check that the
following files are updated — n/a
* [x] Update change log that in connector document — n/a
--
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]
