DanielLeens commented on PR #11512:
URL: https://github.com/apache/seatunnel/pull/11512#issuecomment-6116667895

   Thanks for the re-check, @SEZ9. I re-verified each point directly on head 
`9cf8e059b8` (not just from my earlier reviews). F1-F8 were all addressed in 
commits before the dev merges, and the two merges since (`44d6d665a`, 
`9cf8e059b8`) did not touch these files. Pointers per item:
   
   - **F1 (bounded `activeSplits`)**: 
`CdcEnumeratorProgressReport.MAX_ACTIVE_SPLITS = 100` (line 39); the 
constructor retains `subList(0, retainedSplitCount)` (lines 126-129) and sets 
the `activeSplitsTruncated` flag (lines 65-66, 86). Test: 
`CdcProgressModelTest#testEnumeratorReportBoundsActiveSplitDetails`.
   - **F2 (credentials in payloads)**: `docs/en/developer/cdc-progress.md:38` 
states that position payloads must not contain credentials, connection URLs or 
other authentication material.
   - **F3 (enums by name)**: `CdcProgressReportSerializer` writes enums via 
`.name()` and reads them with `readEnum(in, X.class)` (e.g. lines 53, 113, 
215-219). Tests: 
`ReportCdcProgressOperationSerializationTest#testReaderNamedWireFixtureAndWriterOutput`
 and `#testEnumeratorNamedWireFixtureAndWriterOutput` pin the byte fixtures 
with enum names, and `#testEveryProgressOwnerRoundTripsByName` / 
`#testEveryProgressLifecycleRoundTripsByName` cover the round trip.
   - **F4 (runtime-collection docs)**: the "Runtime collection" section (doc 
lines 40-56) now describes the registration-based path: reader reports sampled 
and batched to the coordinator, enumerator reports collected by the coordinator 
from assigned members, `SupportCdcProgress` opt-in filtering, one outstanding 
request per worker.
   - **F5 (implementing connectors)**: doc lines 108-111 list MySQL, 
PostgreSQL, Oracle, SQL Server, DB2 and MongoDB CDC, plus the MySQL/OceanBase 
and openGauss mappings.
   - **F6 (`SNAPSHOT` Javadoc)**: `CdcProgressLifecycle#SNAPSHOT` now reads 
"The reader is reading snapshot splits.", with no enumerator-owned 
discovery/assignment wording.
   - **F7 (count invariants)**: `CdcEnumeratorProgressReport` constructor calls 
`validateCount` for each count field and `validateExactSplitCounts` (lines 
118-123). Tests: 
`CdcProgressModelTest#testEnumeratorReportRejectsInvalidCounts` and 
`#testEnumeratorReportRejectsEachNegativeCountIndependently`.
   - **F8 (immutability)**: the active-split list is copied defensively, and 
`CdcProgressPosition` copies its map into an unmodifiable one. Tests: 
`#testEnumeratorReportDefensivelyCopiesActiveSplits` and 
`#testEnumeratorReportActiveSplitWatermarksAreDeeplyImmutable`.
   
   The one remaining item from my side is non-blocking: the wire codec bounds 
entry and field counts but not individual string length or total frame bytes 
(my Issue 1 from the last review). Happy to defer to your final pass once you 
have re-checked the above.


-- 
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]

Reply via email to