ROX-20122: set gRPC max concurrent streams to 100 #1287
Conversation
|
Images are ready for the commit at c923de3. To use the images, use the tag |
|
/test e2e-tests |
@dcaravel Yes we could make it configurable. BTW since I'm not familiar with Scanner, do you know what the e2e failures are about? Can it be something that this PR caused? |
No not caused by this PR. TL;DR; our tests rely on external data / vuln feeds, when those feeds change our tests may break (like what your seeing) - I'll fix this particular failure in a different PR. |
|
FYI, #1288 has been merged, if bring in those changes here e2e tests should pass. |
|
Created #1289 so we can accept integer env vars |
|
also, I'm thinking we call it ROX_MAX_CONCURRENT_STREAMS. We should coordinate with whatever may be done for the stackrox repo, though |
vladbologa
force-pushed
the
max-concurrent-streams
branch
2 times, most recently
from
604d1b2 to
1685e2d
Compare
@RTann I called it |
|
/test unit-tests |
vladbologa
force-pushed
the
max-concurrent-streams
branch
from
1685e2d to
678ead4
Compare
api/grpc/grpc.go
Outdated
| ) | ||
|
|
||
| func maxGrpcConcurrentStreams() uint32 { | ||
| if maxGrpcConcurrentStreamsSetting.Int() < 0 { |
There was a problem hiding this comment.
Though the gRPC library defaults this to math.MaxUint when 0, the HTTP/2 library defaults it to at least 100 when 0. Can we also default it to 100?
There was a problem hiding this comment.
Are you suggesting that I change < 0 to<= 0?
There was a problem hiding this comment.
Yeah I missed that. I should do it also in stackrox.
api/grpc/grpc.go
Outdated
| ) | ||
|
|
||
| func init() { | ||
| grpcprometheus.EnableHandlingTimeHistogram() | ||
| } | ||
|
|
||
| var ( | ||
| maxGrpcConcurrentStreamsSetting = env.RegisterIntegerSetting("ROX_GRPC_MAX_CONCURRENT_STREAMS", defaultGrpcMaxConcurrentStreams) |
There was a problem hiding this comment.
let's put this in pkg/env/list.go
As part of the mitigation for cve-2023-44487, some projects have decided to reduce MaxConcurrentStreams to 100 (this is the minimum recommended value in the HTTP/2 standard - default is 250).
This PR applies this change to the Scanner gRPC server.