feat: support http stream response #2322

yejialiango · 2023-05-30T12:29:46Z

Issues associated with this PR

Solutions

You should show your solutions about the issues in your PR, including the overall solutions, details and the changes. At this time, Chinese is allowed to describe these.

UT result

Unit Test is needed if the code is changed, your unit test should cover boundary cases, corner cases, and some exceptional cases. And you need to show the UT result.

Benchmark

If your code involves the processing of every request, you should give the Benchmark Result.

Code Style

Make sure Goimports has run
Show Golint result

codecov · 2023-05-30T12:44:07Z

Codecov Report

Patch coverage: 5.92% and project coverage change: -0.41 ⚠️

Comparison is base (7d48a20) 60.45% compared to head (0760783) 60.04%.

❗ Current head 0760783 differs from pull request most recent head f149d8e. Consider uploading reports for the commit f149d8e to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2322      +/-   ##
==========================================
- Coverage   60.45%   60.04%   -0.41%     
==========================================
  Files         422      422              
  Lines       37507    37784     +277     
==========================================
+ Hits        22673    22687      +14     
- Misses      12597    12859     +262     
- Partials     2237     2238       +1

Impacted Files	Coverage Δ
pkg/metrics/upstream.go	`0.00% <ø> (ø)`
pkg/proxy/var.go	`37.86% <ø> (ø)`
pkg/stream/client.go	`0.00% <0.00%> (ø)`
pkg/stream/http/var.go	`90.76% <ø> (ø)`
pkg/types/upstream.go	`60.71% <ø> (ø)`
pkg/types/variable.go	`58.82% <ø> (ø)`
pkg/stream/http/stream.go	`23.55% <1.92%> (-11.68%)`	⬇️
pkg/proxy/downstream.go	`57.42% <22.22%> (-0.39%)`	⬇️
pkg/upstream/cluster/loadbalancer.go	`83.33% <77.77%> (-0.18%)`	⬇️
pkg/upstream/cluster/stats.go	`93.54% <100.00%> (+0.32%)`	⬆️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

feat: support http1 stream response feat: support http1 stream response feat: support http1 stream response feat: support http1 stream response style: simplify code style: simplify code style: simplify code style: simplify code feat: support http1 stream response

huanglu20124 · 2023-06-09T04:31:39Z

我试了合并了一下这部分代码，测试了下载大文件的场景，发现内存占用过高的问题依然存在。流式传输的代码是参考fasthttp的，fasthttp实现了分段的读以及发，但它还是用了一个buffer缓存整个响应body，这样还是避免不了内存占用高的问题。我感觉handleChunk那里还可以优化一下，每次读取一段数据时，写到一个临时buffer里，这个buffer只缓存本次读取的数据，写到输出流以后就可以不管这些数据了

jizhuozhi

Maybe fasthttp need the dst to hold the full http response body, but for our sence we are not focusing on the full http response body, so the buffer is just used to copy some bytes from upstream to mosn, and to write to the downstream.

jizhuozhi · 2023-06-09T05:16:46Z

pkg/stream/http/stream.go

+
+// copy from fasthttp github.com/valyala/fasthttp@v1.40.0/http.go:1374
+func writeBodyToPipe(r *bufio.Reader, maxBodySize int, contentLength int, writeFunc func([]byte) error) (err error) {
+	buf := make([]byte, 0)


Perf: give a initial capacity to avoid frequent growth

jizhuozhi · 2023-06-09T05:18:08Z

pkg/stream/http/stream.go

+}
+
+// copy from fasthttp github.com/valyala/fasthttp@v1.40.0/http.go:1374
+func writeBodyToPipe(r *bufio.Reader, maxBodySize int, contentLength int, writeFunc func([]byte) error) (err error) {


maxBodySize seems always 0, do we need this argument?

jizhuozhi · 2023-06-09T05:19:26Z

pkg/stream/http/stream.go

+		return dst, nil
+	}
+
+	offset := len(dst)


Memory: why should we always append to the end of buffer but not start with 0?

Thanks.I will reevaluate and optimize this.

huanglu20124 · 2023-06-12T07:08:58Z

各位大佬好，我还发现另外一个问题，clientStreamConnection对象将响应body传给serverStream对象的过程中，使用到了mosn的pipeBuffer，clientStreamConnection相当于消费者，serverStream相当于生产者，实测mosn作为入流量的sidecar时，会出现生产速率远大于消费速率的情况（因为本地通信比远程通信快得多），这时候就会导致pipeBuffer不断扩容，最终出现占用内存过大的情况。

yejialiango · 2023-06-12T07:36:57Z

各位大佬好，我还发现另外一个问题，clientStreamConnection对象将响应body传给serverStream对象的过程中，使用到了mosn的pipeBuffer，clientStreamConnection相当于消费者，serverStream相当于生产者，实测mosn作为入流量的sidecar时，会出现生产速率远大于消费速率的情况（因为本地通信比远程通信快得多），这时候就会导致pipeBuffer不断扩容，最终出现占用内存过大的情况。

估计是有两个因素导致的：

初始化pipebuffer容量写了0，导致频繁扩容
当前是读完一堆长度为content-length的数据才往pipebuffer写

这里我试下把writeBodyFixedSize内部回写的方式改一下，改成固定大小流式回写

huanglu20124 · 2023-06-12T10:43:39Z

多谢答复~，不过这两种手段我都试过了，能起到一定优化效果，但没有根本解决pipebuffer扩容的问题。我们这边的场景是用mosn作为sidecar，代理入流量，下载大文件的时候，还是很容易出现pipeBuffer增长过快的问题

huanglu20124 · 2023-06-12T10:49:13Z

pkg/stream/http/stream.go

+		}
+		data := buf[:n]
+		log.Proxy.Debugf(s.ctx, "[stream] [http] [stream response] receive data: %s", string(data))
+		if err = s.connection.conn.Write(buffer.NewIoBufferBytes(data)); err != nil {


这段代码是从pipeBuffer里读取数据并返回给客户端，如果客户端是远端机器的话，这里可能会耗时较久，导致pipeBuffer消费速度远慢于生产速度，然后pipeBuffer会出现增长过快的情况。我的想法是这里能否控制clientStream里写pipeBuffer的速度，或者干脆不使用pipeBuffer，将数据同步方式从异步改为同步，clientStream收到包直接交给ServerStream返回

我理解了下，这里面实际上是需要backpressure（背压）能力吧？当消费速率下降时主动降低生产速率

嗯嗯是的

这里感觉还是要考虑下场景：

如果改成同步模式，数据是堆积在与upstream的连接中的

如果保持现有的方式，使用pipebuffer传输数据，数据是有可能堆积在mosn内部的
@jizhuozhi 大佬有什么好的建议吗？

刚刚看了下fasthttp，在valyala/fasthttp#911 中已经有合入过流式处理的功能了，所以看下能不能直接在这个功能基础上进行增强

多谢，我看看

yejialiango · 2023-06-16T13:13:43Z

这里可以用fasthttp的SetBodyStream搞，我重写下

adamzhoul · 2024-01-08T03:43:49Z

反馈一个问题:

这里wrap了一下OnReceive
实际上，会先调用w.stream.DestroyStream()。这里会将client（conn) 放回到connPool
然后在调用w.streamReceiver.OnReceive ，将数据返回。

mosn/pkg/stream/client.go

Lines 177 to 180 in cafbd5c

    
           func (w *clientStreamReceiverWrapper) OnReceive(ctx context.Context, headers types.HeaderMap, data types.IoBuffer, trailers types.HeaderMap) { 
        
           	w.stream.DestroyStream() 
        
           	w.streamReceiver.OnReceive(ctx, headers, data, trailers) 
        
           }

这就导致一个问题。当有新的请求进来的时候。由于stream返回还没完成。所以会卡在 serve上一次for循环中

func (conn *clientStreamConnection) serve() {
   for {
      sendStreamResponse(s)  // sendStreamResponse
   }
}

导致，新的请求，从连接池获取到conn，发送完数据。但是迟迟没有开始处理response。

taoyuanyuan · 2024-01-08T04:06:57Z

filter怎么支持stream，也是一个问题~

yejialiango · 2024-01-08T04:10:38Z

这里我抽空看看怎么处理

yejialiango force-pushed the master branch from eb02650 to ee7e899 Compare May 31, 2023 05:04

fix: delete unused code

f149d8e

jizhuozhi added kind/enhancement improvement for exist feature, not bug area/proxy area/stream labels Jun 7, 2023

jizhuozhi requested review from antJack, nejisama, doujiang24 and jizhuozhi June 7, 2023 13:15

jizhuozhi requested changes Jun 9, 2023

View reviewed changes

huanglu20124 reviewed Jun 12, 2023

View reviewed changes

jizhuozhi requested a review from taoyuanyuan June 13, 2023 05:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support http stream response #2322

feat: support http stream response #2322

yejialiango commented May 30, 2023 •

edited

codecov bot commented May 30, 2023 •

edited

huanglu20124 commented Jun 9, 2023

jizhuozhi left a comment •

edited

jizhuozhi Jun 9, 2023

yejialiango Jun 9, 2023

jizhuozhi Jun 9, 2023

jizhuozhi Jun 9, 2023

yejialiango Jun 9, 2023

huanglu20124 commented Jun 12, 2023

yejialiango commented Jun 12, 2023

huanglu20124 commented Jun 12, 2023

huanglu20124 Jun 12, 2023

jizhuozhi Jun 12, 2023

huanglu20124 Jun 12, 2023

yejialiango Jun 12, 2023

jizhuozhi Jun 13, 2023 •

edited

yejialiango Jun 13, 2023

yejialiango commented Jun 16, 2023

adamzhoul commented Jan 8, 2024

taoyuanyuan commented Jan 8, 2024

yejialiango commented Jan 8, 2024

feat: support http stream response #2322

Are you sure you want to change the base?

feat: support http stream response #2322

Conversation

yejialiango commented May 30, 2023 • edited

Issues associated with this PR

Solutions

UT result

Benchmark

Code Style

codecov bot commented May 30, 2023 • edited

Codecov Report

huanglu20124 commented Jun 9, 2023

jizhuozhi left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huanglu20124 commented Jun 12, 2023

yejialiango commented Jun 12, 2023

huanglu20124 commented Jun 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jizhuozhi Jun 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yejialiango commented Jun 16, 2023

adamzhoul commented Jan 8, 2024

taoyuanyuan commented Jan 8, 2024

yejialiango commented Jan 8, 2024

yejialiango commented May 30, 2023 •

edited

codecov bot commented May 30, 2023 •

edited

jizhuozhi left a comment •

edited

jizhuozhi Jun 13, 2023 •

edited