Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] EMFILE: too many open files #485

Open
konradpabjan opened this issue Dec 19, 2023 · 16 comments · May be fixed by actions/toolkit#1723
Open

[bug] EMFILE: too many open files #485

konradpabjan opened this issue Dec 19, 2023 · 16 comments · May be fixed by actions/toolkit#1723
Labels
bug Something isn't working

Comments

@konradpabjan
Copy link
Collaborator

konradpabjan commented Dec 19, 2023

What happened?

Stumbled on a failure that happened with v4. Apparently this doesn't happen with v3

Image

Example run can be found here: https://github.com/spacetelescope/jdaviz/actions/runs/7266578163/job/19798598137

What did you expect to happen?

Successful artifact upload

How can we reproduce it?

Not sure, but I think the key here is a large number of files. In the logs it shows 8824 files which is quite a bit. Maybe some read stream isn't being closed somewhere 🤔

Anything else we need to know?

Nope

What version of the action are you using?

v4

What are your runner environments?

windows

Are you on GitHub Enterprise Server? If so, what version?

No

@konradpabjan konradpabjan added the bug Something isn't working label Dec 19, 2023
@zaikunzhang
Copy link

Also seen in my workflow.

Run actions/upload-artifact@v4.0.0
With the provided path, there will be [7](https://github.com/primalib/prima/actions/runs/7338125515/job/19983281711#step:4:8)3166 files uploaded
Artifact name is valid!
Root directory input is valid!
Beginning upload of artifact content to blob storage
node:events:492
      throw er; // Unhandled 'error' event
      ^

Error: EMFILE: too many open files, open '/home/runner/work/prima/prima/prima_prima_quadruple.1_20.ubln.single.231227_1211_start/HS63'
Emitted 'error' event on ReadStream instance at:
    at emitErrorNT (node:internal/streams/destroy:151:[8](https://github.com/primalib/prima/actions/runs/7338125515/job/19983281711#step:4:9))
    at emitErrorCloseNT (node:internal/streams/destroy:[11](https://github.com/primalib/prima/actions/runs/7338125515/job/19983281711#step:4:12)6:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -24,
  code: 'EMFILE',
  syscall: 'open',
  path: '/home/runner/work/prima/prima/prima_prima_quadruple.1_20.ubln.single.23[12](https://github.com/primalib/prima/actions/runs/7338125515/job/19983281711#step:4:13)27_1211_start/HS63'
}

Node.js v[20](https://github.com/primalib/prima/actions/runs/7338125515/job/19983281711#step:4:21).8.1

@robherley
Copy link
Contributor

We can possibly fix with the file method in archiver:

Appends a file given its filepath using a lazystream wrapper to prevent issues with open file limits.

Right now we are just using the append operation and creating a read stream per file:

@SMoraisAnsys
Copy link

Bumping as I'm facing the same issue on a repo :)

SMoraisAnsys added a commit to ansys/pyaedt that referenced this issue Jan 17, 2024
Problem: it seems that using the new version (v4) of upload-artifact
adds some upper bound on the number of files to be uploaded.

Solution: reverting our actions to use v3 and make changes compatible
with other ansys actions, i.e. changing some ansys/action/... to v4

Associated issue : actions/upload-artifact#485
@sungaila
Copy link

sungaila commented Jan 25, 2024

Same issue here when I tried to upload 9,834 files for a single artifact. The compression-level made no difference.

Had to downgrade to upload-artifact@v3 and download-artifact@v3.

lionel- added a commit to posit-dev/positron that referenced this issue Jan 25, 2024
lionel- added a commit to posit-dev/positron that referenced this issue Jan 25, 2024
SMoraisAnsys added a commit to ansys/pyaedt that referenced this issue Jan 30, 2024
Note: some changes are conflicting with the current full
documentation upload process (cf
actions/upload-artifact#485)

Extra: removing setoutput as its deprecated by github
lionel- added a commit to posit-dev/positron that referenced this issue Feb 1, 2024
@qwerttvv
Copy link

qwerttvv commented Feb 2, 2024

actions/upload-artifact@main
  with:
    name: MSYS
    path: C:\MSYS
    if-no-files-found: warn
    compression-level: 6
    overwrite: false
  env:
    MSYSTEM: MINGW64
With the provided path, there will be 51574 files uploaded
Artifact name is valid!
Root directory input is valid!
Beginning upload of artifact content to blob storage
node:events:492
      throw er; // Unhandled 'error' event
      ^

Error: EMFILE: too many open files, open 'C:\MSYS\msys64\mingw64\lib\python3.11\distutils\tests\__pycache__\test_cygwinccompiler.cpython-311.pyc'
Emitted 'error' event on ReadStream instance at:
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -4066,
  code: 'EMFILE',
  syscall: 'open',
  path: 'C:\\MSYS\\msys64\\mingw64\\lib\\python3.11\\distutils\\tests\\__pycache__\\test_cygwinccompiler.cpython-311.pyc'
}

Node.js v20.8.1

same bug at latest https://github.com/actions/upload-artifact/tree/3a8048248f2f288c271830f8ecf2a1c5d8eb0e9a

actions/upload-artifact@v3 is good but warning

Warning: There are over 10,000 files in this artifact, consider creating an archive before upload to improve the upload performance.

rjanvier added a commit to 3DFin/3DFin that referenced this issue Feb 2, 2024
lionel- added a commit to posit-dev/positron that referenced this issue Feb 2, 2024
mnixry added a commit to mnixry/binutils-wasm that referenced this issue Feb 4, 2024
@zaikunzhang
Copy link

Is this fixed yet?

@ehuelsmann
Copy link

rjanvier added a commit to 3DFin/3DFin that referenced this issue Mar 15, 2024
rjanvier added a commit to 3DFin/3DFin that referenced this issue Mar 16, 2024
* Add a CD system for 3DFin CC Plugin

* Improve build

- fix tests
- improve build times by removing CCViewer
- add upload artifact

* Improve ci build

- Use current 3DFin
- remove duplicated install

* downgrade to upload-artifact v3

- v4 is bugged actions/upload-artifact#485

* Update other build actions

* lower the compression requierment to speed up build

* Use pip and install-qt-action vs. conda

Use venv in CI

Revert the use of venv

python config

...

* Revert to upload artifactv3

zip the archive

* laszip building
SergioVanacloigCarlsberg pushed a commit to carlsberg/malty that referenced this issue Mar 27, 2024
* chore: update storybook config to allow arguments on the URL survive after refresh

* feat: update stories to new format

* feat: add default data test id on images

* Add dataTestid on overlay

* Add dataTestid on paddedContainer

* refactor: migrate checkbox story

* feat: add default data test id to checkbox

* refactor(floater): remove extra line from argTypes

* refactor(hero): use static image to avoid error on image comparison on visual testing

* refactor(price): add data-testId on wrapper

* refactor: update text property from dataQaId to dataTestId

* feat: add dataTestId on progress circle

* feat: add default dataTestId on tooltip

* refactor: update dataTestIds from textarea

* refactor: update dataTestIds from toggle

* refactor: update dataTestIds from select and update story

* chore: update storybook config to allow arguments on the URL survive after refresh

* feat: update stories to new format

* feat: add default data test id on images

* refactor: migrate checkbox story

* feat: add default data test id to checkbox

* Add dataTestid on overlay

* Add dataTestid on paddedContainer

* refactor(floater): remove extra line from argTypes

* refactor(hero): use static image to avoid error on image comparison on visual testing

* refactor(price): add data-testId on wrapper

* refactor: update text property from dataQaId to dataTestId

* feat: add dataTestId on progress circle

* feat: add default dataTestId on tooltip

* refactor: update dataTestIds from textarea

* refactor: update dataTestIds from toggle

* refactor: update dataTestIds from select and update story

* chore: update foregroudColor to use storybook mapping to be able to receive parameters from URL

* chore: bump node to version 18

* refactor: update components to pass dataTestId to Text

* ci: force rebuild

* style: run formatter

* test: include visual test

* wip: trying to create aliased paths for cypress

* refactor: replace js files for ts

* fix: use correct generic type on chainable from cypress to avoid unnecessary any

* chore: enable linting, disable conflicting rules for cypress files and create alias to cypress

* ci: force rebuild

* ci: set node to correct latest

* ci: add dumb visual testing workflow

* ci: add quotes and manual dispatch

* ci: add repository dispatch

* ci: force rebuild

* ci: add dockerfile to run tests locally

* chore: bump cypress to 13:6.4 version

* chore: add script to update images

* ci: use cypress image to run visual testing

* chore: bump node to 20.11

* ci: remove workflow run

* ci: list repository

* Test visual testing PR number

* Test PR-Name

* Test PR-Name v2

* Check title

* Check amplify link

* Run visual testing with amplify link

* Remove -it from docker command

* Use js file extension instead on cypress config

* chore: add type module on package

* ci: install packages before running cypress

* ci: set image on a key

* ci: add step to install deps using bit

* ci: fix environment variable name

* ci: use npm bin to get cypress path

* ci: use bit to install deps

* ci: add bit compile and reuse command on package.json

* ci: add matrix to paralelize runs

* ci: split the jobs into install and testing

* ci: downgrade upload and download action versions

actions/upload-artifact#485

* ci: tar files before upload

* ci: use upload and download action on the same version

* chore: replace every test ot use getFullPageWithVisibleTarget

* feat: wait for fonts to load

* feat: create script to move images from actual to base folder

* feat: update progress circle test and baseline

* revert: rollback base url variable

* Remove useless StyledOption from input phone

* feat: run tests on full screen

* chore: remove `-it` flag

* refactor: remove it.each

* chore: add timer on getFullPageWithVisibleTarget

* refactor: set timer to only 1 ms

* ci: prepare workflow for mobile validation in a different container

* chore: rename base snapshot to force failure

* ci: add steps to upload logs and snapshots when ci fails

* Revert "chore: rename base snapshot to force failure"

This reverts commit 704f2e8.

* chore: delete primary button image to force ci to fail

* Revert "chore: delete primary button image to force ci to fail"

This reverts commit 574c0f5.

* chore: set timer to 100ms

* ci: define retention days

* ci: remove step that upload logs as no log is written

* ci: include wait-on utility to wait for amplify link to be generated

* ci: include a timeout

* refactor: use args to send dataTestId

* chore: update lock file

* test: include negative validation

* Update yarn.lock

* Update yarn.lock with new node version

* Check bit and bvm versions

* Move bit version

* Restrict bvm version

* Rollback yarn.lock file

* Update yarn.lock with older version

---------

Co-authored-by: Sergio Vanacloig <sergiovanacloig97@gmail.com>
@pcfreak30
Copy link

Ditto, just ran into the same problem :/

@rmunn
Copy link

rmunn commented Apr 29, 2024

I've switched actions/toolkit#1723 (proposed fix for retaining file permissions in uploaded .zip files) to use zip.file(file.sourcePath) instead of zip.append(createReadStream(file.sourcePath)) so that that PR can also fix this bug at the same time. It needs a review from repo maintainers before it can go anywhere, though.

@marcodali
Copy link

Is there a command that we can run in the docker container that allow us to increment the number of open file descriptors to 10k lets say for example??

Wouldn't that fix this issue?

CoderDen732 added a commit to CoderDen732/zed that referenced this issue May 20, 2024
@xanather
Copy link

xanather commented Jun 4, 2024

Just found got this issue. Using v3 I get warnings its going to be killed soon aswell:

https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/

This needs to be fixed before that happens @konradpabjan @robherley otherwise it will make it not possible to upload artifacts that contain over the OS file descriptor limit.

@reko-aviary
Copy link

My workaround is to compress first to a .tgz, then upload it. A clunky approach, but seems to do the trick for me. However, I suppose this might be a non-solution for some.

oneJob: 
  steps:
    - name: 🏗 Checkout branch
      uses: actions/checkout@v4
    # ...
    - name: 🗜️ Compress
      run: tar -czf ./upload.tar.gz ./build
    - name: ⬆️ Upload results
      uses: actions/upload-artifact@v4
      with:
        name: build-${{ github.event.number }}
        path: ./upload.tar.gz
        overwrite: true
        if-no-files-found: error
        retention-days: 1 # No need to keep these longer for now, until we can figure if there's a way to share these between jobs
        compression-level: 0 # We are already gzipping above, makes no sense to compress again. Remove wehen upload-artifact fixes its stuff

twoJob:
  needs: oneJob
  steps:
    - name: 🏗 Checkout branch
      uses: actions/checkout@v4
    - name: ⬇️ Download results
      uses: actions/download-artifact@v4
      with:
        name: build-${{ github.event.number }}
    - name: 🤯 Expand
      run: tar -xzf ./upload.tar.gz

@rmunn
Copy link

rmunn commented Jun 21, 2024

@konradpabjan - actions/toolkit#1723 is a PR that should fix this bug (along with #38); it only needs a review from a GitHub team member before it can be merged. Would you have time to take a look at it? It's been nearly two months since I opened the PR, and I'd really like it to not sit there unreviewed for another two months.

@xanather
Copy link

xanather commented Jul 5, 2024

Please merge this before v3 support is removed from GitHub cloud :(

@lczech
Copy link

lczech commented Jul 17, 2024

And another bump from me to increase urgency :-)

@xanather
Copy link

I pay for GitHub enterprise which is quite expensive as a startup. The fact there is file handler leak in v4 on Windows for this long while v3 is about to be killed in November is unacceptable especially since GitHub provides Windows hosted cloud runners themselves.

Code which has fixed the issue has been presented that can either be merged or based on to get this fixed easily.

@robherley, @konradpabjan, @bethanyj28, @jtamsut please prioritize as November is not too far off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.