Add request temp file buffer (#3479) #3659
Conversation
|
Shouldn't the gateway stream the file? Or are you using filters that may be reading the body? |
|
Strictly speaking, the current MVC version of the Gateway doesn't truly support streaming transmission. Its process involves reading the request, caching it, and then sending it out. This approach can lead to Out Of Memory (OOM) issues, as seen in some cases. |
|
This version was built specifically for Spring WebMVC and blocking IO is part of that. When Zuul was supported, there was an option to install it as a servlet outside the spring dispatcher servlet, and that's how large uploads were handled. We don't really have either of those options. This seems like a good middle ground. |
| /** | ||
| * Temp file buffer for request. | ||
| */ | ||
| private boolean fileBufferEnabled = true; |
There was a problem hiding this comment.
This should be disabled by default.
| /** | ||
| * The size threshold which a request will be written to disk. | ||
| */ | ||
| private DataSize fileBufferSizeThreshold = DataSize.ofMegabytes(1L); |
There was a problem hiding this comment.
Let's create a nest class FileBuffer and move these two properties to it.
| @@ -75,4 +75,9 @@ private int copyResponseBodyWithFlushing(InputStream inputStream, OutputStream o | |||
| return totalReadBytes; | |||
| } | |||
|
|
|||
|
|
|||
| protected boolean isWriteClientBodyToFile(Request request) { | |||
| return properties.isFileBufferEnabled() && request.getServerRequest().servletRequest().getContentLength() >= properties.getFileBufferSizeThreshold().toBytes(); | |||
There was a problem hiding this comment.
I wonder if this should include a content type check as well?
| } | ||
| return requestSpec.exchange((clientRequest, clientResponse) -> { | ||
| ServerResponse serverResponse = doExchange(request, clientResponse); | ||
| clearTempFileIfExist(request); |
There was a problem hiding this comment.
This should be guarded by the isWriteClientBodyToFile() check as well.
Add the feature of using temp file as intermediaries when forwarding large requests, which can effectively solve the OOM, and added two configs to control it:
I'm not sure if this solution is appropriate. It may cause some large requests to be slowed down.But I don’t have any other ideas at the moment. This is the solution our team uses. In addition, should the default value of file-buffer-enabled be true?