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

tidb-server options -socket and -host are incompatible #8459

Closed
morgo opened this issue Nov 26, 2018 · 5 comments · Fixed by #8836
Closed

tidb-server options -socket and -host are incompatible #8459

morgo opened this issue Nov 26, 2018 · 5 comments · Fixed by #8836
Assignees

Comments

@morgo
Copy link
Contributor

morgo commented Nov 26, 2018

Bug Report

Please answer these questions before submitting your issue. Thanks!

  1. What did you do?
./bin/tidb-server -socket /tmp/mysql.sock -host 127.0.0.1 -P 3306
  1. What did you expect to see?

I would expect it to listen on both. This is important because of MySQL's special handling of localhost:

  • For the libmysql derived clients it will switch to using unix socket.
  • For other drivers like JDBC it will only ever use tcp.

So it is useful to be able to have the server listen on both for a fully local install.

  1. What did you see instead?

It will only listen on one (the socket!)

  1. What version of TiDB are you using (tidb-server -V or run select tidb_version(); on TiDB)?
mysql> select tidb_version()\G
*************************** 1. row ***************************
tidb_version(): Release Version: v2.1.0-beta-644-g89cd59d21
Git Commit Hash: 89cd59d217637f5e79fdc81681c98531c668e688
Git Branch: newmaster
UTC Build Time: 2018-11-26 04:47:59
GoVersion: go version go1.11 linux/amd64
Race Enabled: false
TiKV Min Version: 2.1.0-alpha.1-ff3dd160846b7d1aed9079c389fc188f7f5ea13e
Check Table Before Drop: false
1 row in set (0.00 sec)
@morgo morgo added type/compatibility type/bug The issue is confirmed as a bug. labels Nov 26, 2018
@morgo
Copy link
Contributor Author

morgo commented Nov 26, 2018

I looked at server/server.go:

// NewServer creates a new Server.
func NewServer(cfg *config.Config, driver IDriver) (*Server, error) {
..
        if cfg.Socket != "" {
                if s.listener, err = net.Listen("unix", cfg.Socket); err == nil {
                        log.Infof("Server is running MySQL Protocol through Socket [%s]", cfg.Socket)
                }
        } else {
                addr := fmt.Sprintf("%s:%d", s.cfg.Host, s.cfg.Port)
                if s.listener, err = net.Listen("tcp", addr); err == nil {
                        log.Infof("Server is running MySQL Protocol at [%s]", addr)
                }
        }

For simplicity, would it be okay to forward the unix socket to the tcp address? Unix socket is not a major use case for TiDB, and this solution looks quite straight forward to add.

@morgo morgo removed the type/bug The issue is confirmed as a bug. label Nov 26, 2018
@morgo
Copy link
Contributor Author

morgo commented Nov 27, 2018

PTAL @jackysp

@jackysp
Copy link
Member

jackysp commented Nov 27, 2018

I found the same thing some days before, and got the suggestion that it would not be fixed. I can't quite remember the reason. Please give me some time to find out the reason.

@morgo
Copy link
Contributor Author

morgo commented Nov 27, 2018

@jackysp OK :-) I can fix it if you are okay with the suggested fix (redirection to tcp socket). It is an important fix for allowing local development (homebrew, dbdeployer) and have some apps use JDBC with others using PHP etc.

@jackysp
Copy link
Member

jackysp commented Nov 28, 2018

@morgo , I discussed this with @coocood . It was not supported because it was not important at that time. Now that it has a high priority, I think we can support it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants