SEZ9 commented on PR #11512: URL: https://github.com/apache/seatunnel/pull/11512#issuecomment-6245293800
Following up on the items from my earlier pass, for re-check against `9cf8e059b8`. I have not re-verified each one against that head, so where something is already addressed, a short note is all I need. **Robustness (`seatunnel-api/src/main/java/org/apache/seatunnel/api/cdc/CdcEnumeratorProgressReport.java`)** - F1: `activeSplits` accepted an unbounded list, which contradicts the documented "bounded active-split details" contract. Please confirm a cap is enforced (fail fast or truncate with a documented policy) so a connector cannot exhaust coordinator memory/transport via this field. - F7: count fields accepted negative and mutually inconsistent values. A constructor-level invariant check (non-negative, plus the obvious relationships between the counts) would be enough. - F8: the "immutable per-split details" claim relies on a shallow copy. Either make `CdcSnapshotSplitProgress` itself immutable or soften the Javadoc so it does not promise more than the class guarantees. **Security (`docs/en/developer/cdc-progress.md`)** - F2: please add an explicit rule that connector-native position payloads must not contain credentials or connection secrets, since they are shipped to and stored on the coordinator. **Compatibility (`seatunnel-api/src/main/java/org/apache/seatunnel/api/cdc/CdcProgressAccuracy.java`)** - F3: since these enums travel in the wire report payload, please confirm the explicit engine codec encodes them by name rather than ordinal, so future enum additions or reordering cannot corrupt decoded reports. **Docs (`docs/en/developer/cdc-progress.md`, `seatunnel-api/src/main/java/org/apache/seatunnel/api/cdc/CdcProgressLifecycle.java`)** - F4: the runtime-collection section described a pull/derive model, while the PR implements a registration-based enumerator report path. Please confirm the doc now matches the actual flow. - F5: please state which connectors currently implement the progress provider. - F6: the `SNAPSHOT` Javadoc includes enumerator-owned discovery/assignment, which contradicts the ownership rule in the new doc. Please reconcile one or the other. A per-item note (addressed / won't fix with rationale) would let me close these out quickly. Thanks! <!-- streview-comment:1842 --> -- 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]
