Fix connection problems, smaller changes

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

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 {

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"))

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 <pcdummy@ThinkPad-T410.pcdummy.lan>  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 <support@webmeisterei.com>  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 <support@webmeisterei.com>  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 <support@webmeisterei.com>  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 <support@webmeisterei.com>  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 <support@webmeisterei.com>  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
 

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

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

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)
 }

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)