andygrove opened a new pull request, #6338: URL: https://github.com/apache/datafusion-comet/pull/6338
Backport of #6108 to `branch-1.0`. #6108 merged to `main` as an empty commit (df8e1533d). #6156 had already made the same change to `captureWritePlan` on 09-23, when this flake failed the Spark 4.2 nightly. So this PR ports the `captureWritePlan` hunk of #6156 (dd68a531c). The rest of #6156 corrects a Spark 3.4 fallback that #6041 added, and #6041 is not on `branch-1.0`. `branch-1.1` needs nothing. It was cut after #6156, and its `CometParquetWriterTestBase.scala` is byte-identical to the one on `main`. ## Which issue does this PR close? None. #6075 is closed on `main` by #6108. ## Rationale for this change `branch-1.0` still has the flaky helper. `captureWritePlan` registers a `QueryExecutionListener` without first draining the asynchronous listener bus, then returns the first matching plan it sees. If an earlier write's end event is still queued, the helper returns that write's plan, and the test reports that the write under test fell back to Spark. That is the failure described in #6075. On `branch-1.0`, 21 of the suite's 30 tests write their seed or input data without the native writer just before the write under test: - 12 complex-type tests, through `writeComplexTypeData`. - The 6 `SaveMode` tests, through `materializeAsCometSource`. Two of them also seed the target. - The 3 basic-write tests, through `createTestData`. ## What changes are included in this PR? `captureWritePlan` in `CometParquetWriterSuite` now drains the listener bus with `CometListenerBusUtils.waitUntilEmpty`, once before it registers its listener and again after the write. It holds the plan in an `AtomicReference` and no longer polls for up to 15 s. The only adaptation is the file. On `main` the helper lives in `CometParquetWriterTestBase`, which #5821 introduced and which is not on `branch-1.0`. The added and removed lines are identical to #6156's hunk. `CometListenerBusUtils` is already on `branch-1.0`. ## How are these changes tested? This changes a test helper only. I ran it locally on `branch-1.0` with the default Spark 4.1 profile and JDK 17: - All 30 tests in `CometParquetWriterSuite` pass. Spotless, scalastyle and scalafix (`-Psemanticdb -Pspark-3.5 -Pscala-2.12`) are clean. - I reproduced the race on `branch-1.0` with a throwaway test that isn't included here. It adds a `SparkListener` that sleeps 2 s on each `SparkListenerSQLExecutionStart`, so the shared listener queue falls behind the writes. It then runs the body of `SaveMode.Append adds new files alongside existing data`. With the old helper it fails with `Expected exactly one CometNativeWriteExec in the plan, but found 0`, because the captured plan is the Comet-disabled `materializeAsCometSource` write to `source.parquet`. With this change it passes. The PR build runs `CometParquetWriterSuite` on all five Spark profiles on Linux and on macOS. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
