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

Stop using file system sync api #4069

Closed
twoeths opened this issue May 26, 2022 · 2 comments · Fixed by #3797
Closed

Stop using file system sync api #4069

twoeths opened this issue May 26, 2022 · 2 comments · Fixed by #3797
Assignees

Comments

@twoeths
Copy link
Contributor

twoeths commented May 26, 2022

Describe the bug

As clearly stated in https://nodejs.org/api/fs.html#synchronous-example, The synchronous APIs block the Node.js event loop and further JavaScript execution until the operation is complete so we should only use it in testing context

Expected behavior

Switch to equivalent async api

@twoeths
Copy link
Contributor Author

twoeths commented May 26, 2022

@twoeths twoeths self-assigned this May 26, 2022
@dapplion
Copy link
Contributor

Yes we should not use the fs sync api for recurring actions doing busy times. That would be only the db, which does not use the sync api. However, for script-ish tasks, like starting the validator cli and reading keystores it really doesn't matter in my opinion
The process is not gonna do anything else until it has read the keystores, so it doesn't matter if it's sync or async for a performance point of view.

The only usage of the fs sync api that's happening during node's high load is:

  • persisting SSZ invalid objects (can be an issue, large files, frequently)
  • persisting ENR (not an issue, tiny file, infrequently)

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 a pull request may close this issue.

2 participants