From e9bf5fddd1e19313dea91b9335dd30ff274e5e73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Jochum?= Date: Thu, 1 Oct 2020 04:54:52 +0200 Subject: [PATCH] Fix connection problems, smaller changes --- README.md | 2 +- cmd/localserver.go | 19 ++++++++++++++++++- cmd/sshserver.go | 19 +++++++++++++++++-- debian/changelog | 19 +++++++++++++------ debian/lql-api@.service | 2 +- internal/gncp/pool.go | 16 ++++++++-------- internal/gncp/pool_test.go | 38 +++++++++++++++++++++++++++++++++++++- lql/client.go | 20 ++++++++++++++------ 8 files changed, 109 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 2cb30e9..449ae0d 100644 --- a/README.md +++ b/README.md @@ -169,7 +169,7 @@ Next create /etc/lql-api/`site`, with the following contents: ```bash LISTEN="localhost:8080" -DEBUG="" +ARGS="" ``` Now you can start the lql-api diff --git a/cmd/localserver.go b/cmd/localserver.go index 7a46317..f0d52d7 100644 --- a/cmd/localserver.go +++ b/cmd/localserver.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "os" "os/signal" "strings" @@ -22,6 +23,7 @@ func init() { localServerCmd.Flags().StringP("htpasswd", "t", "/opt/omd/sites/{site}/etc/htpasswd", "htpasswd file") localServerCmd.Flags().BoolP("debug", "d", false, "Enable Debug on stderr") localServerCmd.Flags().StringP("listen", "l", ":8080", "Address to listen on") + localServerCmd.Flags().StringP("logfile", "f", "/opt/omd/sites/{site}/var/log/lql-api.log", "Logfile to log to") rootCmd.AddCommand(localServerCmd) } @@ -34,8 +36,23 @@ Requires a local lql unix socket.`, Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { sReplacer := strings.NewReplacer("{site}", args[0]) + + logfile, err := cmd.Flags().GetString("logfile") + if err != nil { + fmt.Println(err) + os.Exit(1) + } + logfile = sReplacer.Replace(logfile) + + f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0600) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + defer f.Close() + logger := log.New() - logger.SetOutput(os.Stderr) + logger.SetOutput(f) if !cmd.Flag("debug").Changed { logger.SetLevel(log.InfoLevel) } else { diff --git a/cmd/sshserver.go b/cmd/sshserver.go index 8e16aec..6667831 100644 --- a/cmd/sshserver.go +++ b/cmd/sshserver.go @@ -32,6 +32,7 @@ func init() { sshServerCmd.Flags().StringP("ssh-keyfile", "k", "~/.ssh/id_rsa", "Keyfile") sshServerCmd.Flags().StringP("ssh-password", "p", "", "Password") sshServerCmd.Flags().StringP("listen", "l", ":8080", "Address to listen on") + sshServerCmd.Flags().StringP("logfile", "f", "lql-api-{site}.log", "Logfile to log to") rootCmd.AddCommand(sshServerCmd) } @@ -56,9 +57,24 @@ Examples: `, Args: cobra.ExactArgs(2), Run: func(cmd *cobra.Command, args []string) { + sReplacer := strings.NewReplacer("{site}", args[0]) + + logfile, err := cmd.Flags().GetString("logfile") + if err != nil { + fmt.Println(err) + os.Exit(1) + } + logfile = sReplacer.Replace(logfile) + + f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0600) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + defer f.Close() logger := log.New() - logger.SetOutput(os.Stderr) + logger.SetOutput(f) if !cmd.Flag("debug").Changed { logger.SetLevel(log.InfoLevel) } else { @@ -70,7 +86,6 @@ Examples: logger.WithField("error", err).Error() return } - sReplacer := strings.NewReplacer("{site}", args[0]) destSocket = sReplacer.Replace(destSocket) localSocket := sReplacer.Replace(path.Join(os.TempDir(), "lql-{site}-client.sock")) diff --git a/debian/changelog b/debian/changelog index 9a43c7f..56479e7 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,4 +1,11 @@ -lql-api (0.0.7) UNRELEASED; urgency=medium +lql-api (0.0.8) UNRELEASED; urgency=low + + * Remove debug from the README install + * Finaly fix reconnections + + -- pcdummy Wed, 30 Sep 2020 22:10:05 +0200 + +lql-api (0.0.7) UNRELEASED; urgency=low * Rename socket -> lql-{site}-client.socket * Add filtering @@ -11,31 +18,31 @@ lql-api (0.0.7) UNRELEASED; urgency=medium -- Webmeisterei Support Wed, 30 Sep 2020 16:15:17 +0200 -lql-api (0.0.6) UNRELEASED; urgency=medium +lql-api (0.0.6) UNRELEASED; urgency=low * Use GncpPool.Put, maybe fixes connection problems -- Webmeisterei Support Wed, 30 Sep 2020 05:24:30 +0200 -lql-api (0.0.5) UNRELEASED; urgency=medium +lql-api (0.0.5) UNRELEASED; urgency=low * Rename host -> hosts in stats/tactical_overview -- Webmeisterei Support Wed, 30 Sep 2020 05:24:30 +0200 -lql-api (0.0.4) UNRELEASED; urgency=medium +lql-api (0.0.4) UNRELEASED; urgency=low * Add /v1/table/:name and /v1/table/:name/columns -- Webmeisterei Support Wed, 30 Sep 2020 05:24:30 +0200 -lql-api (0.0.3) UNRELEASED; urgency=medium +lql-api (0.0.3) UNRELEASED; urgency=low * Add ping API -- Webmeisterei Support Tue, 30 Sep 2020 01:31:46 +0200 -lql-api (0.0~git20200929.ca3764e-1) UNRELEASED; urgency=medium +lql-api (0.0~git20200929.ca3764e-1) UNRELEASED; urgency=low * Initial release diff --git a/debian/lql-api@.service b/debian/lql-api@.service index 764afef..45715fd 100644 --- a/debian/lql-api@.service +++ b/debian/lql-api@.service @@ -7,7 +7,7 @@ EnvironmentFile=/etc/lql-api/%i Type=simple User=%i Group=%i -ExecStart=/usr/bin/lql-api localserver %i --listen $LISTEN $DEBUG +ExecStart=/usr/bin/lql-api localserver %i --listen "$LISTEN" $ARGS [Install] WantedBy=multi-user.target \ No newline at end of file diff --git a/internal/gncp/pool.go b/internal/gncp/pool.go index c611158..2fe4eb6 100644 --- a/internal/gncp/pool.go +++ b/internal/gncp/pool.go @@ -150,7 +150,7 @@ func (p *GncpPool) Close() error { return nil } -// Put can put connection back in connection pool. If connection has been closed, the conneciton will be close too. +// Put can put connection back in connection pool. If connection has been closed, the connection will be close too. func (p *GncpPool) Put(conn net.Conn) error { if p.isClosed() == true { return errPoolIsClose @@ -177,22 +177,22 @@ func (p *GncpPool) isClosed() bool { return ret } -// RemoveConn let connection not belong connection pool.And it will close connection. +// Remove let connection not belong connection pool. And it will close connection. func (p *GncpPool) Remove(conn net.Conn) error { if p.isClosed() == true { return errPoolIsClose } - p.lock.Lock() - p.totalConnNum = p.totalConnNum - 1 - p.lock.Unlock() switch conn.(type) { case *CpConn: + // Destroy calls pool.Remove, so do not decrease the number of connections here return conn.(*CpConn).Destroy() default: + p.lock.Lock() + p.totalConnNum = p.totalConnNum - 1 + p.lock.Unlock() return conn.Close() } - return nil } // createConn will create one connection from connCreator. And increase connection counter. @@ -200,11 +200,11 @@ func (p *GncpPool) createConn() (net.Conn, error) { p.lock.Lock() defer p.lock.Unlock() if p.totalConnNum >= p.maxConnNum { - return nil, fmt.Errorf("Connot Create new connection. Now has %d.Max is %d", p.totalConnNum, p.maxConnNum) + return nil, fmt.Errorf("Connot Create a new connection. Pool now has %d conns. Max is %d", p.totalConnNum, p.maxConnNum) } conn, err := p.connCreator() if err != nil { - return nil, fmt.Errorf("Cannot create new connection.%s", err) + return nil, fmt.Errorf("Cannot create a new connection: %s", err) } p.totalConnNum = p.totalConnNum + 1 return conn, nil diff --git a/internal/gncp/pool_test.go b/internal/gncp/pool_test.go index de1edfb..42d637a 100644 --- a/internal/gncp/pool_test.go +++ b/internal/gncp/pool_test.go @@ -134,7 +134,7 @@ func TestRemoveConn(t *testing.T) { conn1, err := pool.Get() err = pool.Remove(conn1) if err != nil { - assert.Fail("Cannot remoce connection.") + assert.Fail("Cannot remove connection.") } err = conn1.Close() if err != nil { @@ -142,6 +142,42 @@ func TestRemoveConn(t *testing.T) { } assert.Fail("Need connection already removed error.") } + +func TestGetAllRemoveAndGetNew(t *testing.T) { + assert := assert.New(t) + maxConns := 3 + pool, err := NewPool(1, maxConns, connCreator) + if err != nil { + assert.Fail("Init conn pool failed") + } + + // Get and remove all connections + for i := 0; i < maxConns; i++ { + conn, err := pool.Get() + if err != nil { + assert.Fail("Get conn failed") + } + _, err = conn.Write([]byte(fmt.Sprintf("Test conn%d", i))) + if err != nil { + assert.Fail("Write message failed") + } + err = pool.Remove(conn) + if err != nil { + assert.Fail("Cannot remove connection") + } + } + + // Now try to get another connection and write to it + conn, err := pool.Get() + if err != nil { + assert.Fail("Get conn failed") + } + _, err = conn.Write([]byte("Test conn1")) + if err != nil { + assert.Fail("Write message failed") + } +} + func connCreator() (net.Conn, error) { return net.Dial("tcp", Host+":"+Port) } diff --git a/lql/client.go b/lql/client.go index 0030f13..b4de60f 100644 --- a/lql/client.go +++ b/lql/client.go @@ -175,38 +175,46 @@ func (c *Client) RequestRaw(context context.Context, request, outputFormat, auth c.logger.WithField("request", request).Debug("Writing request") _, err = conn.Write([]byte(request)) if err != nil && !errors.Is(err, syscall.EPIPE) { + c.logger.WithField("error", err).Debug("Removing failed connection") + c.pool.Remove(conn) return nil, err } else if errors.Is(err, syscall.EPIPE) { - conn.Close() - conn.(*gncp.CpConn).Destroy() + c.pool.Remove(conn) // Destroy -> Create Connections until we don't get EPIPE. numTries := 0 + maxTries := c.pool.GetMaxConns() * 2 for errors.Is(err, syscall.EPIPE) { - c.logger.WithField("error", err).Debug("Trying to reconnect") + c.logger.WithFields(log.Fields{"error": err, "num_tries": numTries, "max_tries": maxTries, "max_conns": c.pool.GetMaxConns()}).Debug("Trying to reconnect") conn, err = c.pool.GetWithContext(context) if err != nil { + // Failed to get a connection, bailout return nil, err } _, err = conn.Write([]byte(request)) if err != nil && !errors.Is(err, syscall.EPIPE) { + // Other error than EPIPE, bailout + c.logger.WithField("error", err).Debug("Removing failed connection") + c.pool.Remove(conn) return nil, err + } else if err == nil { + // We are fine now + break } - conn.Close() c.pool.Remove(conn) numTries++ - if numTries >= (c.pool.GetMaxConns() * 2) { + if numTries >= maxTries { c.logger.WithField("error", err).Error("To much retries can't reconnect") // Bailout to much tries return nil, err } } } - defer conn.Close() + defer c.pool.Put(conn) tmpBuff := make([]byte, 1024) n, err := conn.Read(tmpBuff)