sarutak commented on PR #59840:
URL: https://github.com/apache/spark/pull/59840#issuecomment-6493170737

   Thanks for the fix. The explicit per-test timeout looks good to me, but I 
have a concern about the concurrent reads.
   
   `Promise.all(scalaFiles.map(async (file) => readFile(...)))` opens all 
~6,500 Scala files at once. libuv's threadpool bounds concurrent syscalls, not 
open file descriptors: every `open()` completes before the corresponding reads 
and closes get scheduled, so almost every file is open at the same time. 
Sampling `/dev/fd` while the scan runs, I saw a peak of about 6,500 open fds. 
With a hard fd limit of 4096 (`ulimit -Hn 4096`), the nonce test fails with 
`EMFILE: too many open files`, while the current sequential version passes even 
with `ulimit -Hn 256`. CI passes because the GitHub runner's limit is higher, 
but some developer environments still use a 4096 hard limit, and the number of 
Scala files only grows.
   
   Since the 60s timeout alone already removes the flakiness, could you drop 
the concurrent-read part and keep `readFileSync`? If you want to keep the 
speedup, please bound the concurrency (e.g. process the files in fixed-size 
batches) instead of opening everything at once.
   
   For reference, I found two occurrences of this flake on fork CI: 
https://github.com/Ma77Ball/spark/actions/runs/40570940333/job/122425515632 
(both tests timed out) and 
https://github.com/szehon-ho/spark/actions/runs/40007567459/job/120600314519 
(only the nonce test timed out). In the first run, the `img-src` test also 
exceeded 5s even though it reads only about 20 CSS files, so the time there 
went into the repository-wide glob or a slow runner rather than the file reads. 
This also suggests the timeout, not the concurrent reads, is what fixes the 
flake. Could you add these links to the JIRA or the PR description, and update 
the description accordingly, since it currently says reading concurrently is 
what keeps the wall-clock under the limit?
   


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

Reply via email to