transport: add a draining state check before creating streams #6142
Conversation
| var wg sync.WaitGroup | ||
| // Expect the failure for all the follow-up streams because ct has been closed gracefully. | ||
| for i := 0; i < 200; i++ { | ||
| wg.Add(1) |
There was a problem hiding this comment.
Nit: can you switch the docstring for this code block from "Expect the failure for all the follow-up streams because ct has been closed gracefully." to something like "expect errors trying to create new streams after the client transport has been Gracefully Closed (and is in draining)".
| t.Errorf("_.Read(_) = _, %v, want _, %v or %v", err, errStreamDrain, ErrConnClosing) | ||
| } | ||
| t.Errorf("_.NewStream(_, _) = _, %v, want _, %v", err, ErrConnClosing) | ||
| }() |
There was a problem hiding this comment.
Nit: can you add a top level docstring to this test explaining what this is actually testing (I know it wasn't there previously? I see ct.GracefulClose(), new streams not being able to be created, and a write and a read (expecting io.EOF) from the already created stream. I'm assuming move "// The stream which was created before graceful close can still proceed." from before the wg.Wait (which is waiting on the failed streams for loop) to before the ct.Write. Please add top level docstring that explains this test is gracefully closing a client transport and expecting these two downstream effects (new stream creation fails, and also previously created streams still continue to be able to be operated on and are still functional (all the time or in certain conditions/timebound etc.?)).
2 participants
Fixes #3686
RELEASE NOTES: