-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TCP_Proxy: onLowWatermark crash #3639
Comments
One more backtrace on a similar crash. This one also shows an Assert failure which probably caused the abort?
|
@ggreenway are you interesting in taking a look at this? I think it's just an instance of calling watermark callbacks on an already dead upstream connection. cc @alyssawilk |
@mattklein123 , is there any way that we can unblock ourselves on this issue? We are on-holding a major rollout of Envoy in our fleet. |
@lap1817 you could debug and fix the issue. Or, if you don't care about watermarks you could increase the limits by a lot such that they are never reached? |
@mattklein123 , What is the limit (parameter) that I can raise to avoid watermark? And what does waterMark do? |
per_connection_buffer_limit_bytes control how much each network connection is willing to buffer. |
Looking at this, it looks like we reset file_event_ on closeSocket. readDisable() appears to assume the connection is disconnected and references file_event_ without nullptr checks. There's probably some better fix in tcp proxy session to not call this callback in the shutdown state but I think a simple check in readDisable() to return if file_event_ is null would probably address this crash. I doubt I can dig into the tcp proxy weirdness but if Matt would be amenable to the quick fix I can put something together tomorrow and see if it works well enough for your use casel |
thanks @alyssawilk ! It will be great if we can have a build that avoids the crash. Meanwhile, I will try increase buffer size (though I am a bit concerned about OOM that may come with it) |
…nnection (#3669) Hopefully changing an outstanding tcp proxy session crash to a more minor ASSERT failure Risk Level: Low Testing: unit test of new code. integration tests which fail to repro the underlying bug. Docs Changes: n/a Release Notes: none Hopefully ameliorates #3639 Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
OK, theoretical workaround submitted. Can you folks try a head build and see if it stops the crashing? |
…nnection (envoyproxy#3669) Hopefully changing an outstanding tcp proxy session crash to a more minor ASSERT failure Risk Level: Low Testing: unit test of new code. integration tests which fail to repro the underlying bug. Docs Changes: n/a Release Notes: none Hopefully ameliorates envoyproxy#3639 Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk thanks! will try it today |
Title: TCP_Proxy: onLowWatermark crash
Description:
Envoy Build SHA
Repro steps:
The text was updated successfully, but these errors were encountered: