test: measures the max receive rate and adds a test to ensure the max rate is observed #1119
Conversation
p2p/conn/connection_test.go
Outdated
| require.Nil(t, err) | ||
| defer serverConn.Stop() //nolint:errcheck // ignore for tests | ||
|
|
||
| msgSize := int(cnfg.RecvRate) |
There was a problem hiding this comment.
Don't we want a message size to be greater than the RecvRate so we can assert that MConnection correctly throttles the messages to that peer
There was a problem hiding this comment.
The reason of why msgSize equal to cnfg.RecvRate would serve the purpose of this test is that the traffic rate is actually enforced roughly for every 100 ms (with the max traffic rate of RecvRate/10), and, roughly speaking, the peak rate is also measured based on the traffic rate observed in each 100 ms intervals. As such, a message size configured with cnfg.RecvRate can still lead to a sudden increase in traffic for a 100 ms interval hence resulting in a higher peak rate than allowed, which is exactly the purpose of this test. However, these specifics may not be immediately apparent when examining the test. Therefore, I agree that it would be more sensible to set the message size to a higher value to prevent any potential confusion. Going to update it.
There was a problem hiding this comment.
Please see d71e935
Closes #1111