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

Add support for MySQL UNIX Socket connections #3565

Merged
merged 2 commits into from
Jan 23, 2025

Conversation

NormB
Copy link
Member

@NormB NormB commented Jan 23, 2025

Summary

This PR adds support for MySQL UNIX Socket connections.

Details

OpenSIPS traditionally communicates with MySQL using the network stack. Often times OpenSIPS runs within a Docker container. When MySQL also runs within a Docker container on the same server a performance improvement can be achieved by removing the network stack communication and replacing it with UNIX sockets.

Solution

MySQL supports UNIX socket communication and its C API exposes the mysql_real_connect() function contains a "unix_socket" parameter. Prior to this PR, the "unix_socket" parameter is hard-coded to NULL indicating that is is not used.

This PR enhances the db_url parsing logic to add the ability to specify a UNIX Socket named path. When a UNIX Socket is specified, the "host" and "port" parameters are no longer parsed by the db_url parsing logic. Note that when a UNIX Socket is specified, the MySQL documentation indicates that the "host" must be set to NULL or "localhost". This PR sets the "host" to "localhost" when a UNIX Socket is specified.

The db_url parsing will recognize the following db_url modparam as shown in this example:

modparam("usrloc", "db_url", "mysql://opensips:opensipsrw@unix(/var/run/mysqld/mysqld.sock)/opensips)

The UNIX Socket is defined in the same place that previously would have been the "host:port". The rest of the db_url syntax remains unchanged.

To use a MySQL UNIX Socket in a Docker containerized environment, the following example settings can be made:

Update the MySQL my.cnf file to use a UNIX Socket:

[mysqld]
socket=/var/run/mysqld/mysqld.sock

Update docker-compose.yml to share the UNIX Socket between the MySQL and OpenSIPS services:

volumes:
shared-sockets:
driver: local

services:
mysql:
volumes:
- shared-sockets:/var/run/mysqld

opensips:
volumes:
- shared-sockets:/var/run/mysqld

Define the UNIX Socket in opensips.cfg:

modparam("usrloc", "db_url", "mysql://opensips:opensipsrw@unix(/var/run/mysqld/mysqld.sock)/opensips)

Compatibility

This is a new feature so there are no backward compatibility issues.

The db_url parsing has been enhanced to support the UNIX Socket specification. If not specified, the existing db_url parsing logic will be used and remains unchanged.

Closing issues

@liviuchircu liviuchircu self-assigned this Jan 23, 2025
@liviuchircu liviuchircu added this to the 3.6-dev milestone Jan 23, 2025
@liviuchircu
Copy link
Member

Good work, @NormB! I will merge right away -- it remains to be seen whether this introduces any (small) breakage in the regular DB URL parser, but it doesn't seem to be the case. Worst case, if there is a breakage, we should also add a few unit tests under a new db/test/ directory, to be sure we lock in the expected parsing behavior instead of just fixing the breakage.

@liviuchircu liviuchircu merged commit 36a2df6 into OpenSIPS:master Jan 23, 2025
51 checks passed
@NormB
Copy link
Member Author

NormB commented Jan 24, 2025

db_url_tests1.patch
db_url_tests2.patch

These two patches implement some db_url parsing tests.

The tests include typical MySQL URLs with and without UNIX Socket settings.

To make the tests pass, I have added reasonable "default" values when they are not able to be determined from parsing the URL. These defaults have added a bit of code/logic to db_id.c.

I have a couple of comments:

  1. Specifying default values instead of db_url parsing failures are much more user friendly than what we have today. Others may not hold the same opinion.
  2. The tests are pretty simplistic. They start with a fully formed URL and then the URL is trimmed from the right at important "delimiter" locations.
  3. Many more tests are possible, for example specifying all the values, except for a password (ie, a value from the middle of the URI). Yes, MySQL users without passwords are legal (not wise, but legal).
  4. So, I don't know where to move forward with this. On one hand, I could submit a PR, but the logic updates to db_id.c around default values is something I think you and the rest of the team should come to an agreement on first.

@NormB NormB mentioned this pull request Jan 24, 2025
@NormB NormB deleted the unix_socket branch January 28, 2025 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants