scylladb / gocql

Package gocql implements a fast and robust ScyllaDB client for the Go programming language.
https://docs.scylladb.com/stable/using-scylla/drivers/cql-drivers/scylla-go-driver.html
BSD 3-Clause "New" or "Revised" License
181 stars 57 forks source link

RFE: add option to connect to UNIX socket #175

Closed Michal-Leszczynski closed 3 months ago

Michal-Leszczynski commented 4 months ago

Similar to https://github.com/scylladb/python-driver/issues/278. Having this in gocql would also benefit Scylla Manager - it would allow SM to query DESC SCHEMA or get list of view tables by connecting to Scylla maintenance_socket. It is an important part of making SM compatible with new Scylla 6.0.

I tried to make it work with a custom dialer, but failed: https://github.com/scylladb/scylla-manager/issues/3831#issuecomment-2106877132.

  1. Could we get this implemented?
  2. How much time would it take to implement it? Since this is important for SM to be compatible with Scylla 6.0, it would be good to know whether it's possible to have this before 6.0 release, or should SM be looking for some temporary workarounds.

cc: @avelanarius @sylwiaszunejko @piodul @mykaul

piodul commented 4 months ago

@avelanarius ping

martin-sucha commented 4 months ago

I would prefer if this worked with the custom dialer. There is only a single place that casts to *net.TCPAddr and it sets the default port. Wouldn't the following patch work (I haven't tried to run it)?

diff --git a/control.go b/control.go
index 3806f45..5a21baf 100644
--- a/control.go
+++ b/control.go
@@ -270,7 +270,11 @@ type connHost struct {
 func (c *controlConn) setupConn(conn *Conn) error {
        // we need up-to-date host info for the filterHost call below
        iter := conn.querySystemLocal(context.TODO())
-       host, err := c.session.hostInfoFromIter(iter, conn.host.connectAddress, conn.conn.RemoteAddr().(*net.TCPAddr).Port)
+       defaultPort := 9042
+       if tcpAddr, ok := conn.conn.RemoteAddr().(*net.TCPAddr); ok {
+               defaultPort = tcpAddr.Port
+       }
+       host, err := c.session.hostInfoFromIter(iter, conn.host.connectAddress, defaultPort)
        if err != nil {
                return err
        }

If it works, the main thing that would need to be done is adding a test to prevent future regressions.

roydahan commented 4 months ago

I added it to @sylwiaszunejko queue so it will be part of next sprint planning (starts on May 30th). IIUC, the target for it is SM that should support Scylla 6.0 which is at least a month from now (very optimistic), so the timeframe should fit. @Michal-Leszczynski please let me know if it make sense in terms of time frames.

mykaul commented 4 months ago

IIUC, the target for it is SM that should support Scylla 6.0 which is at least a month from now (very optimistic), so the timeframe should fit.

Not at least. We'd like to release it as soon as possible.