fix: polishing tmp fs and s3 fs #444
Conversation
| @@ -534,6 +541,9 @@ globalThis.bootstrapSBEdge = (opts, extraCtx) => { | |||
| 'writeTextFile': true, | |||
| 'readFile': true, | |||
| 'readTextFile': true, | |||
| 'mkdir': true, | |||
| 'makeTempDir': true, | |||
There was a problem hiding this comment.
hmm..how would makeTempDir works? Does it end up reusing the /tmp configured for the worker? and what if tmpFs is disabled?
There was a problem hiding this comment.
hmm..how would makeTempDir works?
This is tied to the current context: if the path in the actual file system of tmpfs is /tmp/.xmeow, then the result of makeTempDir must be /tmp/.xmeow/.xmeowmeow.
and what if tmpFs is disabled?
Currently, tmpfs is unconditionally set to be enabled, but if it is not, FsError::Unsupported will be returned by prefixfs.
There was a problem hiding this comment.
so I assume the dir option in makeTempDir has no effect? But users can provide prefix and suffix? https://docs.deno.com/api/deno/~/Deno.MakeTempOptions
There was a problem hiding this comment.
I'll show you how this works with a screencast shortly.
There was a problem hiding this comment.
2024-11-22.8.14.23.mov
cc @laktek
| arg!(--"request-buffer-size" <BYTES>) | ||
| .help("The buffer size of the stream that is used to forward a request to the worker") | ||
| .value_parser(value_parser!(u64)) | ||
| .default_value("16384"), |
There was a problem hiding this comment.
I noticed when the flag isn't set, server defaults to 1KiB. Should that be changed to 16KiB as well?
There was a problem hiding this comment.
I noticed when the flag isn't set, server defaults to 1KiB. Should that be changed to 16KiB as well?
I don't think there is a good default value for this, all I can tell you is that making it too small will increase the amount of wasted user worker CPU time because the buffer will have to be emptied from time to time if the request is too large.
There was a problem hiding this comment.
Prior to this PR, this value was fixed at 1024 bytes, and I suspect that the amount of CPU time lost when requests with very large payloads were sent was not small.
|
🎉 This PR is included in version 1.62.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What kind of change does this PR introduce?
Enhancement
Description
This PR is a follow-up to previous #429, finalizing the s3fs implementation and adding integration tests and testing in CI.