[qa] Fix racy barrier in Iceberg snapshot tests - #6752
Conversation
test_iceberg_ordered_snapshot_ingests_all_rows failed in CI with "assert 0 == 60" on python-multihost. The tests waited on input_connector_iceberg_phase == 2 and then queried the table, but the phase gauge is a reader-side signal: crates/iceberg/src/input.rs sets it one statement after queueing the last records, and push_entry only appends to a mutex-guarded deque that the circuit thread drains later. Multihost loses this race where single-host wins it: the coordinator advances last_committed_step, the only step an ad-hoc query may lease, only after every host reports a committed transaction, whereas a single-host pipeline republishes the snapshot in the thread that just committed. Fix: Wait for total_completed_records to catch up with total_input_records. Signed-off-by: Leonid Ryzhyk <ryzhyk@gmail.com>
| return int(rows[0]["c"]) | ||
|
|
||
|
|
||
| def _all_records_completed(pipeline) -> bool: |
There was a problem hiding this comment.
should this be a generic pipeline function?
swanandx
left a comment
There was a problem hiding this comment.
LGTM!
ideally we should use completion token for this kind of verifications right ? e.g. pipeline.wait_for_token(pipeline.generate_completion_token(TABLE, CONNECTOR))
yes, completion tokens work too. The approach in this commit only works because the pipeline stops after ingesting bounded inputs. |
|
test_now failed because seemingly it could not start the pipeline. I had increased the resource requirements because it kept failing. We could make the input smaller. |
|
I can't tell whether https://github.com/feldera/feldera/actions/runs/30494117799/job/90720857158 shows infra issues or product issues. The docker build failed, which indicates infra issues; an integration test failed in a weird way, which might be either. |
test_iceberg_ordered_snapshot_ingests_all_rows failed in CI with "assert 0 == 60" on python-multihost. The tests waited on input_connector_iceberg_phase == 2 and then queried the table, but the phase gauge is a reader-side signal: crates/iceberg/src/input.rs sets it one statement after queueing the last records, and push_entry only appends to a mutex-guarded deque that the circuit thread drains later.
Multihost loses this race where single-host wins it: the coordinator advances last_committed_step, the only step an ad-hoc query may lease, only after every host reports a committed transaction, whereas a single-host pipeline republishes the snapshot in the thread that just committed.
Fix: Wait for total_completed_records to catch up with total_input_records.
Describe Manual Test Plan
Checklist
Breaking Changes?
Mark if you think the answer is yes for any of these components:
Describe Incompatible Changes