Support configuring `MaxConcurrentStreams` for http2 #14169

ahrtr · 2022-06-27T21:46:32Z

Continue to work on 14081 .

Related issues:

Etcd unexpected request time consumption with HTTPs v3 API #14077
ETCD unusual long request when proxying from the grpc gateway #14065
when etcd node have many http long polling connections, it may cause etcd to respond slowly to http requests. #14185

Benchmark test result:

Note I only created some concurrent streams, and there is no any traffic. The test were performed on a Linux VM with 8GiB memory and 4 CPUs (Intel(R) Xeon(R) CPU E5-2680 v4 @ 2.40GHz).

It seems the memory used by concurrent streams isn't very high, it's about 90MB for 1000 concurrent streams, and 108MB for 2000 concurrent streams.

0 stream

# HELP process_resident_memory_bytes Resident memory size in bytes.
# TYPE process_resident_memory_bytes gauge
process_resident_memory_bytes 1.7567744e+07
# HELP process_virtual_memory_bytes Virtual memory size in bytes.
# TYPE process_virtual_memory_bytes gauge
process_virtual_memory_bytes 1.1481223168e+10
# HELP process_virtual_memory_max_bytes Maximum amount of virtual memory available in bytes.
# TYPE process_virtual_memory_max_bytes gauge
process_virtual_memory_max_bytes 1.8446744073709552e+19

# HELP process_cpu_seconds_total Total user and system CPU time spent in seconds.
# TYPE process_cpu_seconds_total counter
process_cpu_seconds_total 0.44

0_stream.txt

3 streams

# HELP process_resident_memory_bytes Resident memory size in bytes.
# TYPE process_resident_memory_bytes gauge
process_resident_memory_bytes 1.994752e+07
# HELP process_virtual_memory_bytes Virtual memory size in bytes.
# TYPE process_virtual_memory_bytes gauge
process_virtual_memory_bytes 1.1481223168e+10
# HELP process_virtual_memory_max_bytes Maximum amount of virtual memory available in bytes.
# TYPE process_virtual_memory_max_bytes gauge
process_virtual_memory_max_bytes 1.8446744073709552e+19

# HELP process_cpu_seconds_total Total user and system CPU time spent in seconds.
# TYPE process_cpu_seconds_total counter
process_cpu_seconds_total 1.55

3_stream.txt

100 streams

# HELP process_resident_memory_bytes Resident memory size in bytes.
# TYPE process_resident_memory_bytes gauge
process_resident_memory_bytes 2.7332608e+07
# HELP process_virtual_memory_bytes Virtual memory size in bytes.
# TYPE process_virtual_memory_bytes gauge
process_virtual_memory_bytes 1.1481485312e+10
# HELP process_virtual_memory_max_bytes Maximum amount of virtual memory available in bytes.
# TYPE process_virtual_memory_max_bytes gauge
process_virtual_memory_max_bytes 1.8446744073709552e+19

# HELP process_cpu_seconds_total Total user and system CPU time spent in seconds.
# TYPE process_cpu_seconds_total counter
process_cpu_seconds_total 0.69

100_streams.txt

1000 streams

# HELP process_resident_memory_bytes Resident memory size in bytes.
# TYPE process_resident_memory_bytes gauge
process_resident_memory_bytes 9.1369472e+07
# HELP process_virtual_memory_bytes Virtual memory size in bytes.
# TYPE process_virtual_memory_bytes gauge
process_virtual_memory_bytes 1.1552464896e+10
# HELP process_virtual_memory_max_bytes Maximum amount of virtual memory available in bytes.
# TYPE process_virtual_memory_max_bytes gauge
process_virtual_memory_max_bytes 1.8446744073709552e+19

# HELP process_cpu_seconds_total Total user and system CPU time spent in seconds.
# TYPE process_cpu_seconds_total counter
process_cpu_seconds_total 1.68

1000_streams.txt

2000 streams

# HELP process_resident_memory_bytes Resident memory size in bytes.
# TYPE process_resident_memory_bytes gauge
process_resident_memory_bytes 1.0809344e+08
# HELP process_virtual_memory_bytes Virtual memory size in bytes.
# TYPE process_virtual_memory_bytes gauge
process_virtual_memory_bytes 1.155272704e+10
# HELP process_virtual_memory_max_bytes Maximum amount of virtual memory available in bytes.
# TYPE process_virtual_memory_max_bytes gauge
process_virtual_memory_max_bytes 1.8446744073709552e+19

# HELP process_cpu_seconds_total Total user and system CPU time spent in seconds.
# TYPE process_cpu_seconds_total counter
process_cpu_seconds_total 1.83

2000_streams.txt

codecov-commenter · 2022-06-27T22:25:36Z

Codecov Report

Merging #14169 (8b6c8b4) into main (248f072) will decrease coverage by 0.18%.
The diff coverage is 77.14%.

@@            Coverage Diff             @@
##             main   #14169      +/-   ##
==========================================
- Coverage   75.40%   75.21%   -0.19%     
==========================================
  Files         455      456       +1     
  Lines       36859    36892      +33     
==========================================
- Hits        27792    27749      -43     
- Misses       7335     7395      +60     
- Partials     1732     1748      +16

Flag	Coverage Δ
all	`75.21% <77.14%> (-0.19%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
server/config/config.go	`79.76% <ø> (ø)`
server/embed/serve.go	`60.08% <40.00%> (-0.91%)`	⬇️
server/etcdmain/grpc_proxy.go	`58.89% <60.00%> (+0.01%)`	⬆️
pkg/flags/uint32.go	`100.00% <100.00%> (ø)`
server/embed/config.go	`73.64% <100.00%> (+0.06%)`	⬆️
server/embed/etcd.go	`75.14% <100.00%> (+0.14%)`	⬆️
server/etcdmain/config.go	`86.23% <100.00%> (+0.11%)`	⬆️
server/etcdserver/api/v3rpc/grpc.go	`91.42% <100.00%> (ø)`
client/v3/leasing/util.go	`88.33% <0.00%> (-10.00%)`	⬇️
client/pkg/v3/fileutil/lock_linux.go	`72.22% <0.00%> (-8.34%)`	⬇️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 248f072...8b6c8b4. Read the comment docs.

ahrtr · 2022-06-28T02:10:40Z

cc @ptabor @serathius @spzala Please take a look.

The fix may also need to be cherry picked to 3.5.

spzala

@ahrtr thanks, nice work. I have few nits.

pkg/flags/uint32.go

server/config/config.go

server/embed/etcd.go

tests/e2e/v3_curl_maxstream_test.go

ahrtr · 2022-07-05T03:20:31Z

@spzala Resolved all your comments, PTAL, thx.

server/embed/config.go

@ahrtr

There is no update on the original PR (see below) for more then 2 weeks. So Benjamin(@ahrtr) continues to work on the PR. The first step is to rebase the PR, because there are lots of conflicts with the main branch. The change to go.mod and go.sum reverted, because they are not needed. The e2e test cases are also reverted, because they are not correct. ``` etcd-io#14081 ``` Signed-off-by: nic-chen <[email protected]> Signed-off-by: Benjamin Wang <[email protected]>

The golang buildin package `flag` doesn't support `uint32` data type, so we need to support it via the `flag.Var`. Signed-off-by: Benjamin Wang <[email protected]>

The default max stream is 250 in http2. When there are more then 250 streams, the client side may be blocked until some previous streams are released. So we need to support configuring a larger `MaxConcurrentStreams`. Signed-off-by: Benjamin Wang <[email protected]>

Signed-off-by: Benjamin Wang <[email protected]>

server/etcdmain/help.go

pkg/flags/uint32_test.go

spzala

lgtm
Thanks @ahrtr

… max value for each client. Signed-off-by: Benjamin Wang <[email protected]>

Signed-off-by: Benjamin Wang <[email protected]>

ahrtr · 2022-07-06T21:14:35Z

lgtm Thanks @ahrtr

Thanks @spzala .

cc @serathius Do you have any other comments on this? I only updated some messages (a vs each) since your last review.

Last question

serathius

Talked with @wojtek-t and we concluded this change is ok for main and backport to v3.5

nic-chen · 2022-07-12T13:27:35Z

Thank you all for making this change, it helps a lot for Apache APISIX users!

…each client can open at a time Also refer to etcd-io#14169 (comment) Signed-off-by: Benjamin Wang <[email protected]>

ahrtr mentioned this pull request Jun 30, 2022

when etcd node have many http long polling connections, it may cause etcd to respond slowly to http requests. #14185

Closed

spzala reviewed Jul 5, 2022

View reviewed changes

ahrtr force-pushed the http_max_stream_20220628 branch from f672f28 to 3167e05 Compare July 5, 2022 02:17

serathius reviewed Jul 5, 2022

View reviewed changes

server/embed/config.go Outdated Show resolved Hide resolved

serathius reviewed Jul 5, 2022

View reviewed changes

server/embed/config.go Show resolved Hide resolved

杨金珏 and others added 4 commits July 6, 2022 03:43

add & refine the uint32Value data type

d8347ec

The golang buildin package `flag` doesn't support `uint32` data type, so we need to support it via the `flag.Var`. Signed-off-by: Benjamin Wang <[email protected]>

add e2e test cases to cover the maxConcurrentStreams

f40b676

Signed-off-by: Benjamin Wang <[email protected]>

ahrtr force-pushed the http_max_stream_20220628 branch from 3167e05 to 063db88 Compare July 6, 2022 08:16

spzala reviewed Jul 6, 2022

View reviewed changes

server/etcdmain/help.go Outdated Show resolved Hide resolved

spzala reviewed Jul 6, 2022

View reviewed changes

pkg/flags/uint32_test.go Outdated Show resolved Hide resolved

spzala approved these changes Jul 6, 2022

View reviewed changes

ahrtr added 2 commits July 7, 2022 04:51

update the comment for MaxConcurrentStreams to clearly state it's the…

1a6fe4d

… max value for each client. Signed-off-by: Benjamin Wang <[email protected]>

add unit test for pkg/flags/Uint32FromFlag in uint32_test.go

8b6c8b4

Signed-off-by: Benjamin Wang <[email protected]>

ahrtr force-pushed the http_max_stream_20220628 branch from 063db88 to 8b6c8b4 Compare July 6, 2022 20:52

ahrtr added the backport/v3.5 label Jul 6, 2022

serathius previously approved these changes Jul 7, 2022

View reviewed changes

serathius approved these changes Jul 12, 2022

View reviewed changes

ahrtr merged commit 9c4fe12 into etcd-io:main Jul 12, 2022

ahrtr added a commit to ahrtr/etcd that referenced this pull request Jul 13, 2022

Add flag --max-concurrent-streams to set the max concurrent stream …

437f377

…each client can open at a time Also refer to etcd-io#14169 (comment) Signed-off-by: Benjamin Wang <[email protected]>

This was referenced Jul 13, 2022

[3.5] Support configuring MaxConcurrentStreams for http2 #14219

Merged

https server will not be impacted by the MaxConcurrentStreams settin… #14081

Closed

Etcd unexpected request time consumption with HTTPs v3 API #14077

Closed

ahrtr mentioned this pull request Jul 13, 2022

ETCD unusual long request when proxying from the grpc gateway #14065

Closed

tangcong mentioned this pull request Jul 20, 2022

Plans for v3.4.20 release #14232

Closed

25 tasks

hansedong mentioned this pull request Sep 13, 2022

bug: when apisix starts for a while, its communication with etcd starts to time out apache/apisix#7078

Closed

tjungblu pushed a commit to tjungblu/etcd that referenced this pull request Jul 26, 2023

Add flag --max-concurrent-streams to set the max concurrent stream …

4a5d0a0

…each client can open at a time Also refer to etcd-io#14169 (comment) Signed-off-by: Benjamin Wang <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support configuring `MaxConcurrentStreams` for http2 #14169

Support configuring `MaxConcurrentStreams` for http2 #14169

ahrtr commented Jun 27, 2022 •

edited

Loading

codecov-commenter commented Jun 27, 2022 •

edited

Loading

ahrtr commented Jun 28, 2022 •

edited

Loading

spzala left a comment

ahrtr commented Jul 5, 2022

spzala left a comment

ahrtr commented Jul 6, 2022

serathius left a comment

nic-chen commented Jul 12, 2022

Support configuring MaxConcurrentStreams for http2 #14169

Support configuring MaxConcurrentStreams for http2 #14169

Conversation

ahrtr commented Jun 27, 2022 • edited Loading

Benchmark test result:

0 stream

3 streams

100 streams

1000 streams

2000 streams

codecov-commenter commented Jun 27, 2022 • edited Loading

Codecov Report

ahrtr commented Jun 28, 2022 • edited Loading

spzala left a comment

Choose a reason for hiding this comment

ahrtr commented Jul 5, 2022

spzala left a comment

Choose a reason for hiding this comment

ahrtr commented Jul 6, 2022

serathius left a comment

Choose a reason for hiding this comment

nic-chen commented Jul 12, 2022

Support configuring `MaxConcurrentStreams` for http2 #14169

Support configuring `MaxConcurrentStreams` for http2 #14169

ahrtr commented Jun 27, 2022 •

edited

Loading

codecov-commenter commented Jun 27, 2022 •

edited

Loading

ahrtr commented Jun 28, 2022 •

edited

Loading