Grapesy server kills client with "too many pings" / "too many empty data"

[edsko]

Two manifestations:

• When running with a larger window size and more workers, the Java client gets killed by the grapesy server ("too many pings"). Related: Make ping rate limit configurable kazu-yamamoto/http2#108 .
• Grapesy kvstore (without JSON) against itself, with 16 MB of connection space, crashes with "too many empty data"

[edsko]

Suggestions:

• We should up the ping limit again (the Java client sends a lot of pings)
• Maybe to reproduce the "too many empty", we could lower the limit (hardcoded in http2 currently) and try again; and if we have a better understanding (or maybe even without), we should submit a patch to http2 to make the limit configurable and raise the default (maybe we should just make all limits configurable).

[FinleyMcIlwaine]

I am now seeing the too many empty data in the kvstore benchmark ran with

cabal run grapesy-kvstore -- --benchmark --no-work-simulation

At commit 700138262da29573f45a8344205ee36c50cc0b71 (on edsko/new-http2-architecture). It appears to be somewhat non-deterministic, as slowing things down with some intermediate checks/tracing makes it less likely.

What I have determined so far is that the server is sending empty data frames to the client.

[FinleyMcIlwaine]

I believe this can be closed now, fixed by #183

[edsko]

Reopening, we're seeing "too many empty data" again.

[edsko]

Note: we previously thought we should in principle be able to set the limit for empty DATA to zero; this is not the case: this would only be possible for clients and server handlers who know when they send their final message that it is in fact the final message; this is not always true. It also also not true for clients they terminate with an exception, which will result in stream being closed with an empty DATA frame (see also #209).

[FinleyMcIlwaine]

I have the reproducer on the finley/177 branch. If you run

cabal run test-grapesy -- -p '/Test.Sanity.Exception/'

You will see the empty data exceptions being printed from handleException.

[edsko]

I think we realized what was happening here. The test in Test.Sanity.Exception starts 100 concurrent calls. As written, due to #209, the server handler that was unlucky enough to deal with the client that throws an exception, would itself terminate with HandlerTerminated. Since this is not an "expected exception" in the test suite, all other still ongoing tests would then be killed, resulting in the empty data frames.