http: fix http/1 client low watermark crash#10657
Conversation
|
@mattklein123 thanks! I'll give it a spin with real traffic tomorrow (but if you want to merge before, that's good too). |
|
@htuch @asraa I've re-run TSAN 3 times and it keeps failing on: I tried running this locally in TSAN mode and it passed. I'm at a loss for how my change would effect this test in this way either way. Any ideas here? |
|
Nevermind I did get this to reproduce locally with a non-RBE build. Looking. |
|
Hmm, I think I'm hitting libevent/libevent#984 and #8199, but I don't understand how my patch would effect this. Also, it seems to fail sporadically so maybe tickling a timing issue. @htuch any suggestions? |
|
You could try flake instructions before and after and see if it's statistically worse? |
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
|
@alyssawilk updated w/ merged flake fix. |
alyssawilk
left a comment
There was a problem hiding this comment.
Thanks for tackling this and thanks even more for the "we need to clean this up" comment because we really need to clean that up :-/
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
This is a regression from #10561.
Fixes #10655
Risk Level: Low (for this fix, the original change was/is still medium)
Testing: New UT
Docs Changes: N/A
Release Notes: N/A