DanielLeens commented on PR #11198:
URL: https://github.com/apache/seatunnel/pull/11198#issuecomment-4944509845
Thanks @SEZ9. I rechecked the same head `c15514f59f49` against your
follow-up review and re-walked the current flush path in `CouchbaseWriter`.
On the specific data-loss point, I do not see a reopened source blocker from
that path on the current revision:
```text
write()
-> buffer rows under synchronized guard
prepareCommit()
-> doFlush()
close()
-> cancel timer
-> shutdown scheduler
-> doFlush()
doFlush()
-> synchronized
-> clear buffer only after the write loop completes successfully
```
So the current head already does tie the buffered path to both
`prepareCommit()` and `close()`, and the timer path does not drop buffered rows
on a failed periodic flush because `buffer.clear()` only happens on success.
I do think your docs and backoff follow-ups are reasonable, but on this
unchanged head I would still keep Daniel's source-level conclusion as cleared.
The CI fact did change since my last comment, though: the `Build` run I
referenced earlier is now cancelled rather than queued:
https://github.com/apache/seatunnel/runs/86400709410
So the current Daniel-side conclusion for this same head is:
- no reopened source-side blocker from Daniel on the current revision
- your documentation and contract notes are useful follow-ups, but I am not
reopening them as blockers on this unchanged head
- the remaining gate is getting a fresh Build result on top of the current
head
--
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]
