envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
25.14k stars 4.83k forks source link

New feature: Generic proxy transmit data by block #29740

Closed liming95 closed 11 months ago

liming95 commented 1 year ago

New feature: Generic proxy transmit data by block

We find the new feature, generic proxy in v1.27.0, is only process data (request/response) after its content is received completely. Otherwise, if we transmit data before all data is received, it will lead to the subsequent data loss .The above way of data transmission will cause the huge memory occupation and performance degradation when the data size is much big. So we want to know whether there are other ways to transmit data by generic proxy. And we have tried to add a new interface in the generic proxy for data transmission by block rather than waiting receiving all data. The data transmission by block is to process data as the data is read from rx buffer.

Looking forward to your reply. @KBaichoo @soulxu @wbpcode

soulxu commented 1 year ago

cc @wbpcode also

wbpcode commented 1 year ago

Thanks for your suggestion. This is a reasonable requirement. And I am also want it. The streamline single request/response would be very helpful for big request/response.

But in our prod env, our requests/responses are not that big and this feature will make the L7 filter chain very complex. So, this feature only has a low priority on my list.

Marked this with help wanted in case the community wants to help it.

liming95 commented 1 year ago

Thank you for replying. We have developed a common interface support this feature by some simple modification and it can work normally in our environment. And I want to know whether we can add these modification in the project and I also want to know what is the problem of these modification in other scenarios . The following is crucial component of our modification.

1.add the new interace onDecodingSuccess(). void onDecodingSuccess(ResponsePtr response, ExtendedOptions options, bool end_stream);

2. when the above function is called, the succeeding operation for clearing ‘stream’ will only be executed when the end_stream is true. For example,

void UpstreamRequest::onDecodingSuccess(ResponsePtr response, ExtendedOptions options, bool end_stream) { if (endstream) { clearStream(options.drainClose()); } parent.onUpstreamResponse(std::move(response), options); }

Our implementation is for the big response transmission by block and the request still need to receive all request data. The main reason is based on a fact that request is usually small and respond is more big. @wbpcode

wbpcode commented 1 year ago

Happy to see the generic proxy be used in your environment. And any contributions are welcome.

If we want to merge the modifications to the upstream, we need to design the interface more carefully to make sure it's reasonable and meaningful.

Rather than refactor current onDecodingSuccess, I think maybe the better way is provide a set of new methods to provide more fine grained control to the decoding processing.

updated at 2023.09.26: Considering the multiplexing, only a set of new methods also aren't enough.

wbpcode commented 1 year ago

Because I will have some free time in next week. So I improved the priority of this task. But after thinking in more detail, I found it's harder than my initial thought, esp considering the possible multiplexing in some protocols.

I have created a PR for discussion for the possible new codec API. Could you also take a look when you have free time? cc @SoSoSorry to if it meets your requirement.

wbpcode commented 11 months ago

29806