Skip to content

[SPARK-59007][PYTHON][TESTS] Fix flaky Arrow grouped map worker logging tests - #58289

Closed
Yicong-Huang wants to merge 1 commit into
apache:masterfrom
Yicong-Huang:flaky-worker-logs
Closed

[SPARK-59007][PYTHON][TESTS] Fix flaky Arrow grouped map worker logging tests#58289
Yicong-Huang wants to merge 1 commit into
apache:masterfrom
Yicong-Huang:flaky-worker-logs

Conversation

@Yicong-Huang

@Yicong-Huang Yicong-Huang commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Wrap the log read-and-assert step of ApplyInArrowTests.test_apply_in_arrow_with_logging and test_apply_in_arrow_iter_with_logging in @eventually so it polls until the asynchronously captured worker logs are visible. The applyInArrow call that produces the logs stays outside the poll so it runs once.

Why are the changes needed?

Observed on fork CI: the pyspark-sql job failed with [DIFFERENT_ROWS] (100%) where spark.tvf.python_worker_logs() returned no rows instead of the expected two WARNING rows, then re-ran green on the next attempt: https://github.com/Yicong-Huang/spark/actions/runs/32828898783/job/97748219772

Root cause: worker logs are captured asynchronously. Python workers emit log records on stdout; on the JVM side a per-worker RedirectThread (PythonWorkerLogCapture) drains stdout and only saves the log block to the BlockManager once it reads the trailing marker line. That drain runs independently of the query result, which returns over a separate socket channel, so python_worker_logs() invoked right after the query can observe zero blocks. Polling the read side waits out the race.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Built with build/sbt -Phive package and ran python/run-tests --testnames 'pyspark.sql.tests.arrow.test_arrow_grouped_map'; both logging tests and the full module pass.

Was this patch authored or co-authored using generative AI tooling?

No.

@uros-b

uros-b commented Aug 26, 2026

Copy link
Copy Markdown
Member

Thank you @Yicong-Huang and @zhengruifeng!

Yicong-Huang added a commit that referenced this pull request Aug 27, 2026
…g tests

### What changes were proposed in this pull request?

Wrap the log read-and-assert step of `ApplyInArrowTests.test_apply_in_arrow_with_logging` and `test_apply_in_arrow_iter_with_logging` in `eventually` so it polls until the asynchronously captured worker logs are visible. The `applyInArrow` call that produces the logs stays outside the poll so it runs once.

### Why are the changes needed?

Observed on fork CI: the `pyspark-sql` job failed with `[DIFFERENT_ROWS]` (100%) where `spark.tvf.python_worker_logs()` returned no rows instead of the expected two WARNING rows, then re-ran green on the next attempt: https://github.com/Yicong-Huang/spark/actions/runs/32828898783/job/97748219772

Root cause: worker logs are captured asynchronously. Python workers emit log records on stdout; on the JVM side a per-worker `RedirectThread` (`PythonWorkerLogCapture`) drains stdout and only saves the log block to the `BlockManager` once it reads the trailing marker line. That drain runs independently of the query result, which returns over a separate socket channel, so `python_worker_logs()` invoked right after the query can observe zero blocks. Polling the read side waits out the race.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Built with `build/sbt -Phive package` and ran `python/run-tests --testnames 'pyspark.sql.tests.arrow.test_arrow_grouped_map'`; both logging tests and the full module pass.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #58289 from Yicong-Huang/flaky-worker-logs.

Authored-by: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com>
Signed-off-by: Yicong-Huang <17627829+Yicong-Huang@users.noreply.github.com>
(cherry picked from commit 02cb44f)
Signed-off-by: Yicong-Huang <17627829+Yicong-Huang@users.noreply.github.com>
@Yicong-Huang

Copy link
Copy Markdown
Contributor Author

Merge Summary:

Posted by merge_spark_pr.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants