Skip to content

Commit

Permalink
Fix the connection leak and user options sync.
Browse files Browse the repository at this point in the history
- fix the lack of closing the cursor for the query that returned no
rows.
- fix syncing of the user options, as previously those were not
  fetched from the database.
  • Loading branch information
alexeyklyukin committed Nov 27, 2017
1 parent 68bb3cd commit 1ffe98b
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 10 deletions.
23 changes: 18 additions & 5 deletions pkg/cluster/pg.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ import (

const (
getUserSQL = `SELECT a.rolname, COALESCE(a.rolpassword, ''), a.rolsuper, a.rolinherit,
a.rolcreaterole, a.rolcreatedb, a.rolcanlogin,
a.rolcreaterole, a.rolcreatedb, a.rolcanlogin, s.setconfig,
ARRAY(SELECT b.rolname
FROM pg_catalog.pg_auth_members m
JOIN pg_catalog.pg_authid b ON (m.roleid = b.oid)
WHERE m.member = a.oid) as memberof
FROM pg_catalog.pg_authid a
FROM pg_catalog.pg_authid a LEFT JOIN pg_db_role_setting s ON (a.oid = s.setrole AND s.setdatabase = 0::oid)
WHERE a.rolname = ANY($1)
ORDER BY 1;`

Expand Down Expand Up @@ -85,6 +85,9 @@ func (c *Cluster) initDbConn() error {
if finalerr != nil {
return fmt.Errorf("could not init db connection: %v", finalerr)
}
// Limit ourselves to a single connection and allow no idle connections.
conn.SetMaxOpenConns(1)
conn.SetMaxIdleConns(-1)

c.pgDb = conn

Expand Down Expand Up @@ -123,16 +126,26 @@ func (c *Cluster) readPgUsersFromDatabase(userNames []string) (users spec.PgUser
var (
rolname, rolpassword string
rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin bool
memberof []string
roloptions, memberof []string
)
err := rows.Scan(&rolname, &rolpassword, &rolsuper, &rolinherit,
&rolcreaterole, &rolcreatedb, &rolcanlogin, pq.Array(&memberof))
&rolcreaterole, &rolcreatedb, &rolcanlogin, pq.Array(&roloptions), pq.Array(&memberof))
if err != nil {
return nil, fmt.Errorf("error when processing user rows: %v", err)
}
flags := makeUserFlags(rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin)
// XXX: the code assumes the password we get from pg_authid is always MD5
users[rolname] = spec.PgUser{Name: rolname, Password: rolpassword, Flags: flags, MemberOf: memberof}
parameters := make(map[string]string)
for _, option := range roloptions {
fields := strings.Split(option, "=")
if len(fields) != 2 {
c.logger.Warningf("skipping malformed option: %q", option)
continue
}
parameters[fields[0]] = fields[1]
}

users[rolname] = spec.PgUser{Name: rolname, Password: rolpassword, Flags: flags, MemberOf: memberof, Parameters: parameters}
}

return users, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type Config struct {
APIPort int `name:"api_port" default:"8080"`
RingLogLines int `name:"ring_log_lines" default:"100"`
ClusterHistoryEntries int `name:"cluster_history_entries" default:"1000"`
TeamAPIRoleConfiguration map[string]string `name:"team_api_role_configuration" default:"log_statement:'all'"`
TeamAPIRoleConfiguration map[string]string `name:"team_api_role_configuration" default:"log_statement:all"`
PodTerminateGracePeriod time.Duration `name:"pod_terminate_grace_period" default:"5m"`
}

Expand Down
16 changes: 12 additions & 4 deletions pkg/util/users/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (s DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserMap,
r.User.Name = newUser.Name
reqs = append(reqs, r)
}
if !reflect.DeepEqual(dbUser.Parameters, newUser.Parameters) {
if len(newUser.Parameters) > 0 && !reflect.DeepEqual(dbUser.Parameters, newUser.Parameters) {
reqs = append(reqs, spec.PgSyncUserRequest{Kind: spec.PGSyncAlterSet, User: newUser})
}
}
Expand Down Expand Up @@ -95,7 +95,7 @@ func (s DefaultUserSyncStrategy) ExecuteSyncRequests(reqs []spec.PgSyncUserReque
func (strategy DefaultUserSyncStrategy) alterPgUserSet(user spec.PgUser, db *sql.DB) (err error) {
queries := produceAlterRoleSetStmts(user)
query := fmt.Sprintf(doBlockStmt, strings.Join(queries, ";"))
if _, err = db.Query(query); err != nil {
if err = runQueryDiscardResult(db, query); err != nil {
err = fmt.Errorf("dB error: %v, query: %s", err, query)
return
}
Expand All @@ -120,7 +120,7 @@ func (s DefaultUserSyncStrategy) createPgUser(user spec.PgUser, db *sql.DB) (err
}
query := fmt.Sprintf(createUserSQL, user.Name, strings.Join(userFlags, " "), userPassword)

_, err = db.Query(query) // TODO: Try several times
err = runQueryDiscardResult(db, query) // TODO: Try several times
if err != nil {
err = fmt.Errorf("dB error: %v, query: %s", err, query)
return
Expand All @@ -146,7 +146,7 @@ func (s DefaultUserSyncStrategy) alterPgUser(user spec.PgUser, db *sql.DB) (err

query := fmt.Sprintf(doBlockStmt, strings.Join(resultStmt, ";"))

_, err = db.Query(query) // TODO: Try several times
err = runQueryDiscardResult(db, query) // TODO: Try several times
if err != nil {
err = fmt.Errorf("dB error: %v query %s", err, query)
return
Expand Down Expand Up @@ -215,3 +215,11 @@ func quoteParameterValue(name, val string) string {
}
return fmt.Sprintf(`'%s'`, strings.Trim(val, " "))
}

func runQueryDiscardResult(db *sql.DB, sql string) error {
rows, err := db.Query(sql)
if rows != nil {
rows.Close()
}
return err
}

0 comments on commit 1ffe98b

Please sign in to comment.