-
Notifications
You must be signed in to change notification settings - Fork 214
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
Mint Adapter crashes #347
Comments
I've already spent a week investigating this. I've now exceeded my time box to look into it. However, I can try to answer questions as needed. |
Hi @rob-brown! I created a small repo to try to reproduce your issue: https://github.com/Nezteb/absinthe_graphql_playground I'll try to find some time over the next couple weeks to reproduce it. If you have any ideas of how I might reproduce this sooner, feel free to let me know! 😄 |
My main focus was concurrent connections since I thought that was the likely cause. So far it doesn't seem to be the case. There are some other, non-happy-path cases I haven't tested such as connection timeouts, high latency, and clients disconnecting abruptly. I've since seen a few log series like the following in one service:
It appears that throwing an exception in a handler can kill the stream response process, which produced the error I'm seeing. It also caused an exit message to be sent to a GenServer that wasn't expecting it in This is not the typical case. Almost every time I've seen the crash, it was not due to the handler crashing. There were no other related logs with the crash to provide more context. |
Hey @rob-brown Sorry, I haven't been able to check on this repo issues. I'll try to understand what is happening with your code any time in the next weeks (we could even schedule a pair to try to understand this together) I few highlights:
While designing the adapter, I tried to keep the behavior consistent with what If the maintainers agreed, I open to redesign the adapter to be more elixir-wise (with a proper OTP handling of the possible issues), but that will cause the adapters to differ on their behavior. Now, about your exception. What I can tell you is that, something is killing the |
It's been a while since I last looked at this but I never saw any crash reports or logs pointing to the owning processes crashing. That's what I would have expected and looked for first. I'm not aware of any logic that terminates connections early either. |
Describe the bug
I've been testing out
GRPC.Client.Adapters.Mint
. On almost every service we try it, we get crashes like this:To Reproduce
Steps to reproduce the behavior:
Change adapter from
GRPC.Client.Adapters.Gun
toGRPC.Client.Adapters.Mint
.Expected behavior
No crashes.
Logs
Protos
None
Versions:
OS: Ubuntu 22.04.3 LTS
Erlang: 25.1.2
Elixir: 1.14.1
mix.lock(grpc, gun, cowboy, cowlib):
locked at 2.10.0 (cowboy) 3afdccb7
locked at 2.12.1 (cowlib) 163b73f6
locked at 0.7.0 (grpc) 632a9507
locked at 2.0.1 (gun) a10bc8d6
Additional context
I don't have a minimal example to demonstrate the issue. We have one small service that is running the Mint adapter just fine. Any other service we've tried using it has frequent crashes with the above log.
At first, I thought it might be related to #345. However, we already have our own connection pooling in place. After doing some calculations our pool configuration should have 4-10x the capacity necessary.
After auditing the code, I have some questions. First and foremost, why is the Mint adapter trapping exits on a process it doesn't own? My best guess is that it's because it's spawning and linking a couple processes (
GRPC.Client.Adapters.Mint.ConnectionProcess
andGRPC.Client.Adapters.Mint.StreamResponseProcess
) and doesn't want those crashing to bring down the calling process.I don't believe exit trapping is an appropriate choice. From the docs:
Additionally, since trapping exits causes a message to be delivered to the linked process, this can cause other problems. If the process is a
GenServer
and it implementshandle_info
without a catch-all function, then this message will cause theGenServer
to crash. Or, if it's some other process, then the process mailbox will slowly grow over time with these exit messages.Once one or more of these processes crash, what would restart them? The calling process would only know it crashed from the exit signal message but doesn't have the knowledge to restart the processes. Wouldn't it be better to spawn
GRPC.Client.Adapters.Mint.ConnectionProcess
andGRPC.Client.Adapters.Mint.StreamResponseProcess
under aDynamicSupervisor
or equivalent? That would allow connections to restart properly, and it could limit the crashing to the relevant request instead of all requests in a connection.The text was updated successfully, but these errors were encountered: