davidzollo commented on PR #11103:
URL: https://github.com/apache/seatunnel/pull/11103#issuecomment-4894131464
Hi @zhangshenghang, thanks for working on this fix. The synchronized
direction looks helpful for preventing `finishAndCloseFile()` from closing a
stream while `write()` is still using it.
I have one checkpoint-boundary question that may be worth discussing before
merge:
In Zeta, `prepareCommit(barrierId)` and `snapshotState(barrierId)` are
invoked as two separate calls in `SinkFlowLifeCycle`. With this PR, both
methods are synchronized individually, but the monitor is released after
`prepareCommit()` returns and acquired again when `snapshotState()` starts.
For File Sink, `prepareCommit()` closes the current transaction files and
builds the commit info, while `beginTransaction(checkpointId + 1)` still
happens later in `snapshotState()`. So I wonder whether the following
interleaving is still possible if a write thread enters between the two calls:
```text
checkpoint thread:
prepareCommit()
finishAndCloseFile() for transaction T1
return and release lock
write thread:
write()
transactionId is still T1 because snapshotState() has not called
beginTransaction(T2) yet
new data may be written into the already prepared transaction T1
checkpoint thread:
snapshotState()
captures state for T1
beginTransaction(T2)
```
If the engine guarantees that no `write()` can be called between
`prepareCommit()` and `snapshotState()` for the same writer instance, then the
current approach should be fine. But if that guarantee does not hold, would it
be safer to freeze the `FileCommitInfo` / `FileSinkState` and switch to
`beginTransaction(checkpointId + 1)` inside the same synchronized
`prepareCommit(checkpointId)` critical section, then let `snapshotState()` only
return the already frozen state?
This is not a rejection of the synchronized approach; I just want to make
sure the checkpoint boundary is fully protected, not only the individual method
bodies. What do you think?
--
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]
