PT-1859 ( PT-1868 ) and general pt-pg-summary improvements (#455)

* Fix for PT-1868 and general pt-pg-summary improvements

This is a rather large piece of changes to pt-pg-summary, which
includes:
* Corrected dependency for models in lib/pginfo
* Fixed existing testing infrastructure and implemented new tests:
** Test for New in pginfo
** Test for TestCollectGlobalInfo
** Test for TestCollectPerDatabaseInfo
* Fixed models to reflect PG12 changes (datid 0 with no name)
** Modified gen.sh to include PG12 containers and work with the same
docker-compoe that tests use
* Updated templates and helper functions
* Fixed standby detection and template output

With these changes, pt-pg-summary works correctly with PG12 hosts.

* Extra port in pt-pg-summary models gen.sh, removing unused

.gitignore

@@ -24,3 +24,4 @@ src/go/.env
 config/deb/control.bak
 config/rpm/percona-toolkit.spec.bak
 config/sphinx-build/percona-theme/*
+coverage.out

src/go/lib/pginfo/pginfo.go

@@ -5,7 +5,7 @@ import (
 	"regexp"
 	"time"
 
-	"github.com/Percona-Lab/pt-pg-summary/models"
+	"github.com/percona/percona-toolkit/src/go/pt-pg-summary/models"
 	"github.com/hashicorp/go-version"
 	"github.com/pkg/errors"
 	"github.com/shirou/gopsutil/process"
@@ -94,11 +94,11 @@ func new(db models.XODB, databases []string, sleep int, logger *logrus.Logger) (
 
 	serverVersion, err := models.GetServerVersion(db)
 	if err != nil {
-		return nil, errors.Wrap(err, "Cannot get the connected clients list")
+		return nil, errors.Wrap(err, "Cannot get server version")
 	}
 
 	if info.ServerVersion, err = parseServerVersion(serverVersion.Version); err != nil {
-		return nil, fmt.Errorf("cannot get server version: %s", err.Error())
+		return nil, fmt.Errorf("Cannot parse server version: %s", err.Error())
 	}
 	info.logger.Infof("Detected PostgreSQL version: %v", info.ServerVersion)
 
@@ -198,7 +198,7 @@ func (i *PGInfo) CollectGlobalInfo(db models.XODB) []error {
 		}
 	}
 
-	if !i.ServerVersion.LessThan(version10) {
+	if i.ServerVersion.GreaterThanOrEqual(version10) {
 		i.logger.Info("Collecting Slave Hosts (PostgreSQL 10+)")
 		if i.SlaveHosts10, err = models.GetSlaveHosts10s(db); err != nil {
 			errs = append(errs, errors.Wrap(err, "Cannot get slave hosts in Postgre 10+"))

src/go/pt-pg-summary/internal/tu/tu.go

@@ -50,9 +50,9 @@ var (
 	IPv6PG12Port = getVar("PG_IPV6_12_PORT", ipv6PG12Port)
 
 	PG9DockerIP  = getContainerIP(pg9Container)
-	PG10DockerIP = getContainerIP(pg9Container)
+	PG10DockerIP = getContainerIP(pg10Container)
-	PG11DockerIP = getContainerIP(pg9Container)
+	PG11DockerIP = getContainerIP(pg11Container)
-	PG12DockerIP = getContainerIP(pg9Container)
+	PG12DockerIP = getContainerIP(pg12Container)
 
 	DefaultPGPort = "5432"
 )

src/go/pt-pg-summary/main.go

@@ -127,12 +127,33 @@ func connect(dsn string) (*sql.DB, error) {
 
 func funcsMap() template.FuncMap {
 	return template.FuncMap{
-		"trim": func(s string, size int) string {
+		"trim": func(size int, s string) string {
 			if len(s) < size {
 				return s
 			}
-			return s[:size]
+			return s[:size]+"..."
 		},
+        "convertnullstring": func(s sql.NullString) string {
+            if s.Valid {
+                return s.String
+            } else {
+                return ""
+            }
+        },
+        "convertnullint64": func(s sql.NullInt64) int64 {
+            if s.Valid {
+                return s.Int64
+            } else {
+                return 0
+            }
+        },
+        "convertnullfloat64": func(s sql.NullFloat64) float64 {
+            if s.Valid {
+                return s.Float64
+            } else {
+                return 0.0
+            }
+        },
 	}
 }
 

src/go/pt-pg-summary/main_test.go

@@ -6,30 +6,38 @@ import (
 	"testing"
 
 	"github.com/percona/percona-toolkit/src/go/pt-pg-summary/internal/tu"
+	"github.com/percona/percona-toolkit/src/go/lib/pginfo"
+
+	"github.com/sirupsen/logrus"
 )
 
+type Test struct {
+	name     string
+	host     string
+	port     string
+	username string
+	password string
+}
+
+var tests []Test = []Test{
+	{"IPv4PG9", tu.IPv4Host, tu.IPv4PG9Port, tu.Username, tu.Password},
+	{"IPv4PG10", tu.IPv4Host, tu.IPv4PG10Port, tu.Username, tu.Password},
+	{"IPv4PG11", tu.IPv4Host, tu.IPv4PG11Port, tu.Username, tu.Password},
+	{"IPv4PG12", tu.IPv4Host, tu.IPv4PG12Port, tu.Username, tu.Password},
+}
+
+var logger = logrus.New()
+
 func TestMain(m *testing.M) {
+	logger.SetLevel(logrus.WarnLevel)
 	os.Exit(m.Run())
 }
 
 func TestConnection(t *testing.T) {
-	tests := []struct {
+	// use an "external" IP to simulate a remote host
-		name     string
+	tests := append(tests, Test{"remote_host", tu.PG9DockerIP, tu.DefaultPGPort, tu.Username, tu.Password})
-		host     string
+	// use IPV6 for PostgreSQL 9
-		port     string
+	//tests := append(tests, Test{"IPV6", tu.IPv6Host, tu.IPv6PG9Port, tu.Username, tu.Password})
-		username string
-		password string
-	}{
-		{"IPv4PG9", tu.IPv4Host, tu.IPv4PG9Port, tu.Username, tu.Password},
-		{"IPv4PG10", tu.IPv4Host, tu.IPv4PG10Port, tu.Username, tu.Password},
-		{"IPv4PG11", tu.IPv4Host, tu.IPv4PG11Port, tu.Username, tu.Password},
-		{"IPv4PG12", tu.IPv4Host, tu.IPv4PG12Port, tu.Username, tu.Password},
-		// use IPV6 for PostgreSQL 9
-		//{"IPV6", tu.IPv6Host, tu.IPv6PG9Port, tu.Username, tu.Password},
-		// use an "external" IP to simulate a remote host
-		{"remote_host", tu.PG9DockerIP, tu.DefaultPGPort, tu.Username, tu.Password},
-	}
-
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
@@ -42,3 +50,77 @@ func TestConnection(t *testing.T) {
 	}
 
 }
+
+func TestNewWithLogger(t *testing.T) {
+    for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			dsn := fmt.Sprintf("host=%s port=%s user=%s password=%s sslmode=disable dbname=%s",
+				test.host, test.port, test.username, test.password, "postgres")
+			db, err := connect(dsn);
+			if err != nil {
+				t.Errorf("Cannot connect to the db using %q: %s", dsn, err)
+			}
+			if _, err := pginfo.NewWithLogger(db, nil, 30, logger); err != nil {
+				t.Errorf("Cannot run NewWithLogger using %q: %s", dsn, err)
+			}
+		})
+    }
+}
+
+
+func TestCollectGlobalInfo(t *testing.T) {
+    for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			dsn := fmt.Sprintf("host=%s port=%s user=%s password=%s sslmode=disable dbname=%s",
+				test.host, test.port, test.username, test.password, "postgres")
+			db, err := connect(dsn);
+			if err != nil {
+				t.Errorf("Cannot connect to the db using %q: %s", dsn, err)
+			}
+			info, err := pginfo.NewWithLogger(db, nil, 30, logger);
+			if err != nil {
+				t.Errorf("Cannot run NewWithLogger using %q: %s", dsn, err)
+			}
+			errs := info.CollectGlobalInfo(db)
+			if len(errs) > 0 {
+				logger.Errorf("Cannot collect info")
+				for _, err := range errs {
+					logger.Error(err)
+				}
+				t.Errorf("Cannot collect global information using %q", dsn)
+			}
+		})
+    }
+}
+
+func TestCollectPerDatabaseInfo(t *testing.T) {
+    for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			dsn := fmt.Sprintf("host=%s port=%s user=%s password=%s sslmode=disable dbname=%s",
+				test.host, test.port, test.username, test.password, "postgres")
+			db, err := connect(dsn);
+			if err != nil {
+				t.Errorf("Cannot connect to the db using %q: %s", dsn, err)
+			}
+			info, err := pginfo.NewWithLogger(db, nil, 30, logger);
+			if err != nil {
+				t.Errorf("Cannot run New using %q: %s", dsn, err)
+			}
+			for _, dbName := range info.DatabaseNames() {
+				dsn := fmt.Sprintf("host=%s port=%s user=%s password=%s sslmode=disable dbname=%s",
+					test.host, test.port, test.username, test.password, dbName)
+				conn, err := connect(dsn);
+				if err != nil {
+					t.Errorf("Cannot connect to the %s database using %q: %s", dbName, dsn, err)
+				}
+				if err := info.CollectPerDatabaseInfo(conn, dbName); err != nil {
+					t.Errorf("Cannot collect information for the %s database using %q: %s", dbName, dsn, err)
+				}
+				conn.Close()
+			}
+		})
+    }
+}

src/go/pt-pg-summary/models/clusterinfo.xo.go

@@ -12,7 +12,7 @@ import (
 type ClusterInfo struct {
 	Usename        string         // usename
 	Time           time.Time      // time
-	ClientAddr     string         // client_addr
+	ClientAddr     sql.NullString // client_addr
 	ClientHostname sql.NullString // client_hostname
 	Version        string         // version
 	Started        time.Time      // started

src/go/pt-pg-summary/models/counters.xo.go

@@ -27,7 +27,7 @@ func GetCounters(db XODB) ([]*Counters, error) {
 	var err error
 
 	// sql query
-	var sqlstr = `SELECT datname, numbackends, xact_commit, xact_rollback, ` +
+	var sqlstr = `SELECT COALESCE(datname, '') datname, numbackends, xact_commit, xact_rollback, ` +
 		`blks_read, blks_hit, tup_returned, tup_fetched, tup_inserted, ` +
 		`tup_updated, tup_deleted, conflicts, temp_files, ` +
 		`temp_bytes, deadlocks ` +

src/go/pt-pg-summary/models/databases.xo.go

@@ -15,7 +15,8 @@ func GetDatabases(db XODB) ([]*Databases, error) {
 
 	// sql query
 	var sqlstr = `SELECT datname, pg_size_pretty(pg_database_size(datname)) ` +
-		`FROM pg_stat_database`
+		`FROM pg_stat_database ` +
+		`WHERE datid <> 0`
 
 	// run query
 	XOLog(sqlstr)

src/go/pt-pg-summary/models/gen.sh

@@ -3,9 +3,10 @@ USERNAME=postgres
 PASSWORD=root
 PORT9=6432
 PORT10=6433
+PORT12=6435
 DO_CLEANUP=0
 
-if [ ! "$(docker ps -q -f name=pt-pg-summary_postgres9_1)" ]; then
+if [ ! "$(docker ps -q -f name=go_postgres9_1)" ]; then
     DO_CLEANUP=1
     docker-compose up -d --force-recreate
     sleep 20
@@ -53,7 +54,7 @@ xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
 ORDER BY 1
 ENDSQL
 
-FIELDS='Usename string,Time time.Time,ClientAddr string,ClientHostname sql.NullString,Version string,Started time.Time,IsSlave bool'
+FIELDS='Usename string,Time time.Time,ClientAddr sql.NullString,ClientHostname sql.NullString,Version string,Started time.Time,IsSlave bool'
 COMMENT='Cluster info'
 xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
     --query-mode \
@@ -77,7 +78,7 @@ SELECT usename, now() AS "Time",
 ENDSQL
 
 COMMENT="Databases"
-xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
+xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT12}/?sslmode=disable \
     --query-mode \
     --query-trim  \
     --query-interpolate \
@@ -87,6 +88,7 @@ xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
     --out ./ << ENDSQL
 SELECT datname, pg_size_pretty(pg_database_size(datname)) 
   FROM pg_stat_database
+  WHERE datid <> 0
 ENDSQL
  
 xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
@@ -101,14 +103,14 @@ xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
 GROUP BY 1
 ENDSQL
  
-xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
+xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT12}/?sslmode=disable \
     --query-mode \
     --query-interpolate \
     --query-trim  \
     --query-type Counters \
     --package models \
     --out ./ << ENDSQL
-  SELECT datname, numbackends, xact_commit, xact_rollback, 
+  SELECT COALESCE(datname, '') datname, numbackends, xact_commit, xact_rollback,
          blks_read, blks_hit, tup_returned, tup_fetched, tup_inserted, 
          tup_updated, tup_deleted, conflicts, temp_files, 
          temp_bytes, deadlocks 
@@ -116,9 +118,9 @@ xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
 ORDER BY datname
 ENDSQL
 
-FIELDS='Relname string, Relkind string,Datname string,Count sql.NullInt64'
+FIELDS='Relname string, Relkind string, Datname sql.NullString, Count sql.NullInt64'
 COMMENT='Table Access'
-xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
+xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT12}/?sslmode=disable \
     --query-mode \
     --query-trim  \
     --query-type TableAccess \
@@ -128,7 +130,7 @@ xo pgsql://${USERNAME}:${PASSWORD}@127.0.0.1:${PORT9}/?sslmode=disable \
     --query-allow-nulls \
     --package models \
     --out ./ << ENDSQL 
-  SELECT c.relname, c.relkind, b.datname, count(*) FROM pg_locks a 
+  SELECT c.relname, c.relkind, b.datname datname, count(*) FROM pg_locks a
     JOIN pg_stat_database b 
       ON a.database=b.datid 
     JOIN pg_class c 

src/go/pt-pg-summary/models/tableaccess.xo.go

@@ -9,10 +9,10 @@ import (
 
 // Table Access
 type TableAccess struct {
-	Relname string        // relname
+	Relname string         // relname
-	Relkind string        // relkind
+	Relkind string         // relkind
-	Datname string        // datname
+	Datname sql.NullString // datname
-	Count   sql.NullInt64 // count
+	Count   sql.NullInt64  // count
 }
 
 // GetTableAccesses runs a custom query, returning results as TableAccess.
@@ -20,7 +20,7 @@ func GetTableAccesses(db XODB) ([]*TableAccess, error) {
 	var err error
 
 	// sql query
-	var sqlstr = `SELECT c.relname, c.relkind, b.datname, count(*) FROM pg_locks a ` +
+	var sqlstr = `SELECT c.relname, c.relkind, b.datname datname, count(*) FROM pg_locks a ` +
 		`JOIN pg_stat_database b ` +
 		`ON a.database=b.datid ` +
 		`JOIN pg_class c ` +

src/go/pt-pg-summary/templates/templates.go

@@ -5,8 +5,10 @@ var TPL = `{{define "report"}}
 {{ template "tablespaces" .Tablespaces }}
 {{ if .SlaveHosts96 -}}
   {{ template "slaves_and_lag" .SlaveHosts96 }}
-{{ else if .SlaveHosts10 -}}
+{{- else if .SlaveHosts10 -}}
   {{ template "slaves_and_lag" .SlaveHosts10 }}
+{{- else -}}
+  {{ template "slaves_and_log_none" }}
 {{- end }}
 {{ template "cluster" .ClusterInfo }}
 {{ template "databases" .AllDatabases }}
@@ -43,32 +45,33 @@ var TPL = `{{define "report"}}
 ` +
 	`{{ define "slaves_and_lag" -}}
 ##### --- Slave and the lag with Master --- ####
-{{ if . -}}
++----------------------+----------------------+--------------------------------+-------------------+
-+----------------------+----------------------+----------------------------------------------------+
 |  Application Name    |    Client Address    |           State                |      Lag          |
-+----------------------+----------------------+----------------------------------------------------+
++----------------------+----------------------+--------------------------------+-------------------+
 {{ range . -}}` +
-	`| {{ printf "%-20s" .ApplicationName }} ` +
+	`| {{ convertnullstring .ApplicationName | printf "%-20s" }} | ` +
-	`| {{ printf "%-20s" .ClientAddr }} ` +
+	`{{ convertnullstring .ClientAddr | printf "%-20s" }} | ` +
-	`| {{ printf "%-50s" .State }} ` +
+	`{{ convertnullstring .State | printf "%-30s" }} | ` +
-	`|  {{ printf "% 4.2f" .ByteLag }}` +
+	`{{ convertnullfloat64 .ByteLag | printf "% 17.2f" }} |` + "\n" +
-	`{{ end -}} {{/* end define */}}
+	`{{ end -}}
 +----------------------+----------------------+----------------------------------------------------+
-{{- else -}}
+{{ end -}} {{/* end define */}}
+` +
+    `{{- define "slaves_and_log_none" -}}
+##### --- Slave and the lag with Master --- ####
 There are no slave hosts
-{{ end -}}
+{{ end -}} {{/* end define */}}
-{{ end -}}
 ` +
 	`{{ define "cluster" -}}
 ##### --- Cluster Information --- ####
 {{ if . -}}
 +------------------------------------------------------------------------------------------------------+
 {{- range . }}
- Usename        : {{ printf "%-20s" .Usename }}                                               
+ Usename        : {{ trim 20 .Usename }}
  Time           : {{ printf "%v" .Time }}
- Client Address : {{ printf "%-20s" .ClientAddr }}                            
+ Client Address : {{ convertnullstring .ClientAddr | trim 20 }}
- Client Hostname: {{ trim .ClientHostname.String 80 }}                        
+ Client Hostname: {{ convertnullstring .ClientHostname | trim 90 }}
- Version        : {{ trim .Version 80 }}                                      
+ Version        : {{ trim 90 .Version }}
  Started        : {{ printf "%v" .Started }}
  Is Slave       : {{ .IsSlave }}
 +------------------------------------------------------------------------------------------------------+
@@ -97,7 +100,7 @@ Database: {{ $dbname }}
 +----------------------+------------+
 |      Index Name      |    Ratio   |
 +----------------------+------------+
-| {{ printf "%-20s" .Name }} |     {{ printf "% 5.2f" .Ratio.Float64 }}  |
+| {{ printf "%-20s" .Name }} |     {{ convertnullfloat64 .Ratio | printf "% 5.2f" }}  |
 +----------------------+------------+
 {{ else -}}
   No stats available
@@ -144,9 +147,9 @@ Database: {{ $dbname }}
 +----------------------+------------+---------+----------------------+---------+
 {{ range . -}}` +
 	`| {{ printf "%-20s" .Usename }} | ` +
-	`{{ printf "%-20s" .Client.String }} | ` +
+	`{{ convertnullstring .Client | printf "%-20s" }} | ` +
-	`{{ printf "%-20s" .State.String }} | ` +
+	`{{ convertnullstring .State | printf "%-20s" }} | ` +
-	`{{ printf "% 7d" .Count.Int64 }} |` + "\n" +
+	`{{ convertnullint64 .Count | printf "% 7d" }} |` + "\n" +
 	`{{ end -}}
 +----------------------+------------+---------+----------------------+---------+
 {{ else -}}
@@ -266,8 +269,8 @@ Database: {{ $dbname }}
 	`{{ range . -}}
 | {{ printf "%-50s" .Relname }} ` +
 	`|   {{ printf "%1s" .Relkind }}  ` +
-	`| {{ printf "%-30s" .Datname }} ` +
+	`| {{ convertnullstring .Datname | printf "%-30s" }} ` +
-	`| {{ printf "% 7d" .Count.Int64 }} ` +
+	`| {{ convertnullint64 .Count | printf "% 7d" }} ` +
 	"|\n" +
 	"{{ end }}" +
 	"+----------------------------------------------------" +
@@ -286,7 +289,7 @@ Database: {{ $dbname }}
 	"                           Value                     \n" +
 	`{{ range $name, $values := . -}}` +
 	` {{ printf "%-45s" .Name }} ` +
-	`: {{ printf "%-60s" .Setting }}  ` +
+	`: {{ printf "%s" .Setting }}` +
 	"\n" +
 	"{{ end }}" +
 	"{{ end }}" +