SEZ9 commented on PR #13060: URL: https://github.com/apache/seatunnel/pull/13060#issuecomment-6567605147
Thanks for the follow-up and for pushing `ba97d640` plus the Spotless-only `35110606`. Yes, I'll re-review the current head; here is what I need before I can dismiss the outstanding change request. Your summary covers the transaction-ID grammar / restore-parser documentation in `KafkaSinkWriter`, the Awaitility-bounded E2E assertions, and the PR description counts. What I can't yet map from the comment is how each of the previously raised findings landed, so please confirm (a short per-item note is enough): 1. **Flink scale-down restore (F1)** – `validateRestoredStateOwner` previously assumed Zeta's modulo redistribution; Flink's ListState repartitioning hands out contiguous chunks. Your verification lists Zeta 3-to-2 and Spark runs, but no Flink restore. Is the owner check now engine-agnostic, or is it scoped to Zeta only? If the latter, that scope should be stated explicitly. 2. **Unconfigured `transaction_prefix` (F2)** – with per-writer random prefixes, does `restoreState()` still reject mixed prefixes on scale-down, or is that path handled now? 3. **Scale-up writer with no state (F3)** – does a fresh writer still start its sequence at 1 regardless of the checkpoint ID, leaving a failed first attempt unfenced until the next checkpoint? The "contiguous transaction sequence vs. engine checkpoint ID" doc you added sounds related, so please spell out the intended behavior. 4. **Spark task retries (F4)** – since the subtask index is the Spark taskId, how is the previous attempt's open transaction fenced on retry? The Spark batch E2E passing doesn't by itself exercise that. 5. **Concurrent legacy-namespace fencing (F5/F6)** – is the repeated probing across restoring writers still present, or was it changed/deemed acceptable? 6. **Docs (F7)** – was the `transaction_prefix` option row in `docs/en/connectors/sink/Kafka.md` aligned with the new ID-shape text and the engine scope of the rescaling claim? Your changed-file list mentions only two files, so I want to be sure this wasn't dropped. Also, the exact-head run at `41336107891` was still in progress when you posted; please drop a note once it completes. Once I have those answers I'll do the re-review and, if everything checks out, dismiss the change request. <!-- streview-comment:2208 --> -- 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]
