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

Implement the File API's File.close method #3149

Open
oleiade opened this issue Jun 27, 2023 · 1 comment
Open

Implement the File API's File.close method #3149

oleiade opened this issue Jun 27, 2023 · 1 comment
Assignees
Labels
blocked issue's resolution is blocked enhancement

Comments

@oleiade
Copy link
Member

oleiade commented Jun 27, 2023

As part of #3141, implementing the agreed-upon design for k6's File API, we intend to implement a File.close method.

The close method marks a File as closed and unfit for further use. We, however, anticipate its implementation to be a noop at the moment.

However, depending on the implementation itself, it might turn out that we want to make some reference counting of some sort to make sure to free the file resources from memory as soon as no VU interacts with it anymore.

prototype

interface File {
    close()
}

example

const file = fs.openSync("fou.txt")
file.close()
@oleiade
Copy link
Member Author

oleiade commented Oct 25, 2023

We have discussed this topic heavily with @codebien and concluded that in the current state of k6, introducing this API wouldn't bring outstanding benefits and probably wouldn't lead to a better user experience either.

The current implementation of the fs module does not rely on file descriptors under the hood but rather behaves like a filesystem cache, and the benefit of closing files, for that matter, is not immediately apparent.

We have looked into introducing reference-counted file opening/closing. Still, because we are constrained to opening them in the init context at the moment, the performance benefits we wanted to introduce by doing so are unfortunately canceled.

As a result we decided to put the introduction of this API on hold until we have a good enough reason to do so, both from the technical and/or the user-experience perspective. A good technical reason to introduce it in the future might come as a result of #1079.

@oleiade oleiade added the blocked issue's resolution is blocked label Oct 25, 2023
@olegbespalov olegbespalov removed this from the v0.48.0 milestone Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked issue's resolution is blocked enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants