goutamadwant commented on PR #12216:
URL: https://github.com/apache/seatunnel/pull/12216#issuecomment-5841704094

   I ran `TestFilterRowKindIT#testFilterRowKindMultiTable` in a loop locally on 
the Flink 1.15.3 and 1.18.0 containers only, which are the legs that fail in 
CI. I compared `dev` (`deb16a3c3`), this PR (`AssertSinkWriter` patched into 
the connector-assert jar the containers pick up), and #12454 (its `parallelism 
= 1` conf change). The host test JVM was JDK 8. Six CPU burners ran on the host 
for most runs.
   
   | variant | runs (2 Flink invocations each) | failed |
   |---|---|---|
   | `dev` | 14 | 8 (Flink 1.15 ×6, 1.18 ×2) |
   | this PR | 13 | 0 |
   | #12454 conf | 6 | 0 |
   
   The `dev` failures match CI:
   
   - The sink runs as `Writer (x/4)`.
   - The first writer to close fails `MIN_ROW` on a partial total.
   - Flink restarts the task, and the next attempts fail `MAX_ROW` with 125 and 
then 150, because the static counters were never reset.
   
   `AssertSinkWriterCloseTest` passes 5/5 with the change and fails 2/5 against 
the current `AssertSinkWriter`, so it does catch the bug.
   
   Two questions on the "last open writer in this JVM" rule:
   
   1. The count comes from writers *constructed* so far, not from the sink's 
parallelism. If one subtask's writer is created late, after another subtask has 
already closed, the earlier close would still see a partial total. That seems 
unlikely in these tests, but it is possible in principle.
   2. If a failed attempt's writer is never `close()`d before the task 
restarts, its slot in `OPEN_WRITERS` is never released. The rules would then 
never be evaluated for that table. A job that should fail on MIN/MAX_ROW would 
pass silently, where today it fails loudly. A unit test for "writer dropped 
without close, new writer created" would settle it either way. Keying off the 
sink parallelism from the writer context might avoid both cases.
   
   #12454 fixes only this one conf (`parallelism = 1`), and the other 
multi-table Assert confs keep the same exposure. This PR is the more complete 
fix if the two points above hold up.
   


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