slachiewicz commented on PR #883: URL: https://github.com/apache/incubator-xtable/pull/883#issuecomment-5196701034
Closing this — the CI run on this PR shows it makes the build slower, not faster, so the premise behind it is wrong. Leaving the measurement here since the result is useful on its own. `Maven CI Build` hit the workflow's `timeout-minutes: 30` at 30m16s, against 27m22s on the run I took as the baseline. | | before | after | | --- | --- | --- | | Total integration-test time | 3823s | 5842s (+53%) | | The 44 `ITConversionController` invocations | 1003s | 2637s (2.6x) | The reduced `ITConversionController`, holding only `testVariousOperations` — 16 of the original 44 invocations — took **1133s by itself, more than the whole 44-invocation class did before the split**. The decisive part is what happened to classes this PR does not touch at all: ``` ITDeltaConversionSource 684s -> 1002s (1.46x) ITHudiConversionSource 635s -> 830s (1.31x) ITDeltaKernelConversionSource 456s -> 556s (1.22x) ITParquetConversionSource 237s -> 269s (1.13x) ``` Untouched tests do not get 20-46% slower because of a refactor elsewhere. That is resource contention. ## What I got wrong I read `ITConversionController`'s 1003s as evidence of a serialization bottleneck: failsafe runs with `reuseForks=false`, so one class occupies one fork and its tests run serially there regardless of `forkCount`. That part is true, but the conclusion only follows if the fork pool has spare CPU to absorb more concurrency — and it does not. `forkCount=6` already oversubscribes the runner, and every fork is a Spark JVM at `-Xmx1500m`. Splitting into five classes added four more concurrent Spark JVMs and four more `SparkSession` startups to a machine that was already saturated, so everything slowed down, including the other tests. `xtable-core` looks CPU-bound on the runner rather than serialization-bound. A long class time can be a symptom of contention rather than proof of a bottleneck, and I should have checked for spare capacity before assuming otherwise. ## Possibly worth trying instead If the runner really is oversubscribed, the useful direction is the opposite of this PR: **lowering** `forkCount` toward the runner's core count, so six Spark JVMs stop thrashing over four vCPUs. That is a small change, though it needs measuring rather than assuming — I would not want to repeat the mistake in this PR by asserting it. Otherwise the levers that reduce total work rather than rearranging it: a larger runner if ASF infra offers one for this project, or #882, which lets unchanged modules skip their build and tests entirely. -- 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]
