Skip to content
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

Refactor server and proxy #343

Closed
4 tasks done
mostafa opened this issue Sep 30, 2023 · 0 comments · Fixed by #344
Closed
4 tasks done

Refactor server and proxy #343

mostafa opened this issue Sep 30, 2023 · 0 comments · Fixed by #344
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@mostafa
Copy link
Member

mostafa commented Sep 30, 2023

Intro

I tasked myself with benchmarking GatewayD to find and resolve issues and to prove that GatewayD can be used in production environments. I expected some issues, but was not expecting the end results of this benchmark. The connection queues were filling up very quickly, because the proxy was blocking the event-loop via the client code, thus causing TCP Zero Window errors, which subsequently left pgbench hanging and waiting forever. There is actually more to it than I summarized in the last sentence. This led to a lot of profiling, debugging, printing and pulling my hairs (figuratively), which resulted in a few failed attempts at refactoring. This got me thinking that I should fundamentally change the way connections are handled in GatewayD.

Issue

The Server object (and code) is using gnet/v2 underneath, which is a really awesome project with a lot of great features. Those features were why I initially (and up until now) aimed at using it to its full extent. However, there are certain design decisions for the future of GatewayD and a few missing features from gnet/v2 that make it challenging to continue using gnet to serve traffic to database clients, namely TLS support and certain functions, for example Reader.Next, not being thread-safe.

Due to the non-blocking nature of gnet's event-loop, blocking code like client.Receive will block the event loop, which prevents bidirectional communication between the server and the client. I tried to fix this using various methods like using goroutines, channels and other readily-available Go features, but to no avail. Subsequently, this issue also prevents certain SQL features not to work properly, like cancelling a SQL query or the LISTEN/NOTIFY commands in PostgreSQL.

Decision

After lots of back and forth and pondering, my final decision is to refactor the server and the proxy, remove gnet/v2 and replace it with the Go stdlib's net package, which natively supports all the above features (and more). It is thread-safe, and according to the net.Conn docs, I can use goroutines (and channels) to pass traffic around:

Multiple goroutines may invoke methods on a Conn simultaneously.

This makes it easier to mimic the gnet's event-driven API and to replace it smoothly. Plus, TLS is supported out of the box, which can be implemented on both the client connection and the server connections. Bidirectional communication is yet another feature that can be implemented with relative ease.

This is one of the hardest decisions I made in this project, as it involves a lot of code changes and refactoring, yet this will benefit the project both immediately and in the long run.

Related Tickets

@mostafa mostafa added the enhancement New feature or request label Sep 30, 2023
@mostafa mostafa added this to the v0.8.x milestone Sep 30, 2023
@mostafa mostafa self-assigned this Sep 30, 2023
@mostafa mostafa moved this from ✨ New to 📋 Backlog in GatewayD Core Public Roadmap Sep 30, 2023
@mostafa mostafa changed the title Refactor server Refactor server and proxy Sep 30, 2023
@mostafa mostafa moved this from 📋 Backlog to 🚧 In progress in GatewayD Core Public Roadmap Sep 30, 2023
@mostafa mostafa linked a pull request Oct 7, 2023 that will close this issue
12 tasks
@mostafa mostafa moved this from 🚧 In progress to 👀 In review in GatewayD Core Public Roadmap Oct 17, 2023
@mostafa mostafa moved this from 👀 In review to 🔀 Merged in GatewayD Core Public Roadmap Oct 17, 2023
@mostafa mostafa moved this from 🔀 Merged to 🎉 Done in GatewayD Core Public Roadmap Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

1 participant