Skip to content

Commit d9fd3b1

Browse files
davissp14davissp14
authored
File/Conn handling fixes (#121)
* File/Conn handling fixes * don't block * Cleanup * Fix workflow * Ditch matrix * Fix test cmd * Ignore checks pkg * Tuning github workflows
1 parent a6f4847 commit d9fd3b1

18 files changed

+177
-69
lines changed

‎.github/workflows/pre-commit.yaml

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎.github/workflows/push.yml

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
name: "Push"
2+
on:
3+
push:
4+
branches:
5+
- main
6+
pull_request:
7+
types:
8+
- opened
9+
- synchronize
10+
- reopened
11+
12+
jobs:
13+
build:
14+
name: "Build"
15+
runs-on: ubuntu-latest
16+
steps:
17+
- uses: actions/checkout@v3
18+
19+
- uses: actions/setup-go@v3
20+
with:
21+
go-version: '1.19'
22+
go-version-file: 'go.mod'
23+
cache: true
24+
25+
- name: Build binaries
26+
run: go install ./cmd/...
27+
28+
unit:
29+
name: "Unit Tests"
30+
runs-on: ubuntu-latest
31+
strategy:
32+
fail-fast: false
33+
34+
steps:
35+
- uses: actions/checkout@v3
36+
37+
- uses: actions/setup-go@v3
38+
with:
39+
go-version: '1.19'
40+
go-version-file: 'go.mod'
41+
cache: true
42+
43+
- name: Run unit tests
44+
run: go test -v ./...
45+
46+
47+
staticcheck:
48+
name: "Staticcheck"
49+
runs-on: ubuntu-latest
50+
steps:
51+
- uses: actions/checkout@v3
52+
53+
- uses: actions/setup-go@v3
54+
with:
55+
go-version: '1.19'
56+
go-version-file: 'go.mod'
57+
cache: true
58+
59+
- uses: dominikh/[email protected]
60+
with:
61+
install-go: false
62+
63+
errcheck:
64+
name: "Errcheck"
65+
runs-on: ubuntu-latest
66+
steps:
67+
- uses: actions/checkout@v3
68+
69+
- uses: actions/setup-go@v3
70+
with:
71+
go-version: '1.19'
72+
go-version-file: 'go.mod'
73+
cache: true
74+
75+
- run: go install github.com/kisielk/errcheck@latest
76+
- run: errcheck -ignore 'AddCheck' ./...

‎cmd/event_handler/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func processEvent(ctx context.Context) error {
5050
if err != nil {
5151
return fmt.Errorf("failed to open local connection: %s", err)
5252
}
53-
defer conn.Close(ctx)
53+
defer func() { _ = conn.Close(ctx) }()
5454

5555
member, err := node.RepMgr.Member(ctx, conn)
5656
if err != nil {

‎cmd/monitor/monitor_cluster_state.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func clusterStateMonitorTick(ctx context.Context, node *flypg.Node) error {
2525
if err != nil {
2626
return fmt.Errorf("failed to open local connection: %s", err)
2727
}
28-
defer conn.Close(ctx)
28+
defer func() { _ = conn.Close(ctx) }()
2929

3030
member, err := node.RepMgr.Member(ctx, conn)
3131
if err != nil {
@@ -60,5 +60,9 @@ func clusterStateMonitorTick(ctx context.Context, node *flypg.Node) error {
6060
}
6161
}
6262

63+
if err := conn.Close(ctx); err != nil {
64+
return fmt.Errorf("failed to close connection: %s", err)
65+
}
66+
6367
return nil
6468
}

‎cmd/monitor/monitor_dead_members.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func deadMemberMonitorTick(ctx context.Context, node *flypg.Node, seenAt map[int
5656
if err != nil {
5757
return fmt.Errorf("failed to open local connection: %s", err)
5858
}
59-
defer conn.Close(ctx)
59+
defer func() { _ = conn.Close(ctx) }()
6060

6161
member, err := node.RepMgr.MemberByID(ctx, conn, int(node.RepMgr.ID))
6262
if err != nil {
@@ -87,8 +87,15 @@ func deadMemberMonitorTick(ctx context.Context, node *flypg.Node, seenAt map[int
8787

8888
continue
8989
}
90-
defer sConn.Close(ctx)
9190
seenAt[standby.ID] = time.Now()
91+
92+
if err := sConn.Close(ctx); err != nil {
93+
return fmt.Errorf("failed to close connection: %s", err)
94+
}
95+
}
96+
97+
if err := conn.Close(ctx); err != nil {
98+
return fmt.Errorf("failed to close connection: %s", err)
9299
}
93100

94101
return nil

‎cmd/monitor/monitor_replication_slots.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package main
22

33
import (
44
"context"
5+
"fmt"
56
"log"
67
"time"
78

@@ -26,7 +27,7 @@ func replicationSlotMonitorTick(ctx context.Context, node *flypg.Node, inactiveS
2627
if err != nil {
2728
log.Printf("failed to open local connection: %s\n", err)
2829
}
29-
defer conn.Close(ctx)
30+
defer func() { _ = conn.Close(ctx) }()
3031

3132
member, err := node.RepMgr.Member(ctx, conn)
3233
if err != nil {
@@ -82,5 +83,9 @@ func replicationSlotMonitorTick(ctx context.Context, node *flypg.Node, inactiveS
8283
}
8384
}
8485

86+
if err := conn.Close(ctx); err != nil {
87+
return fmt.Errorf("failed to close connection: %s", err)
88+
}
89+
8590
return nil
8691
}

‎cmd/pg_unregister/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func processUnregistration(ctx context.Context) error {
3737
if err != nil {
3838
return fmt.Errorf("failed to connect to local db: %s", err)
3939
}
40-
defer conn.Close(ctx)
40+
defer func() { _ = conn.Close(ctx) }()
4141

4242
member, err := node.RepMgr.MemberByHostname(ctx, conn, string(hostnameBytes))
4343
if err != nil {

‎internal/api/handle_admin.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func handleRole(w http.ResponseWriter, r *http.Request) {
8989
renderErr(w, err)
9090
return
9191
}
92-
defer conn.Close(r.Context())
92+
defer func() { _ = conn.Close(r.Context()) }()
9393

9494
member, err := node.RepMgr.Member(r.Context(), conn)
9595
if err != nil {
@@ -130,7 +130,7 @@ func handleUpdatePostgresSettings(w http.ResponseWriter, r *http.Request) {
130130
renderErr(w, err)
131131
return
132132
}
133-
defer conn.Close(r.Context())
133+
defer func() { _ = conn.Close(r.Context()) }()
134134

135135
consul, err := state.NewStore()
136136
if err != nil {
@@ -210,7 +210,7 @@ func handleApplyConfig(w http.ResponseWriter, r *http.Request) {
210210
renderErr(w, err)
211211
return
212212
}
213-
defer conn.Close(r.Context())
213+
defer func() { _ = conn.Close(r.Context()) }()
214214

215215
consul, err := state.NewStore()
216216
if err != nil {
@@ -241,7 +241,7 @@ func handleViewPostgresSettings(w http.ResponseWriter, r *http.Request) {
241241
renderErr(w, err)
242242
return
243243
}
244-
defer conn.Close(r.Context())
244+
defer func() { _ = conn.Close(r.Context()) }()
245245

246246
var requestedSettings []string
247247
if err := json.NewDecoder(r.Body).Decode(&requestedSettings); err != nil {

‎internal/api/handle_databases.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func handleListDatabases(w http.ResponseWriter, r *http.Request) {
1616
renderErr(w, err)
1717
return
1818
}
19-
defer conn.Close(r.Context())
19+
defer func() { _ = conn.Close(r.Context()) }()
2020

2121
dbs, err := admin.ListDatabases(ctx, conn)
2222
if err != nil {
@@ -41,7 +41,7 @@ func handleGetDatabase(w http.ResponseWriter, r *http.Request) {
4141
renderErr(w, err)
4242
return
4343
}
44-
defer conn.Close(r.Context())
44+
defer func() { _ = conn.Close(r.Context()) }()
4545

4646
db, err := admin.FindDatabase(ctx, conn, name)
4747
if err != nil {
@@ -63,7 +63,7 @@ func handleCreateDatabase(w http.ResponseWriter, r *http.Request) {
6363
renderErr(w, err)
6464
return
6565
}
66-
defer conn.Close(r.Context())
66+
defer func() { _ = conn.Close(r.Context()) }()
6767

6868
var input createDatabaseRequest
6969
if err := json.NewDecoder(r.Body).Decode(&input); err != nil {
@@ -82,7 +82,7 @@ func handleCreateDatabase(w http.ResponseWriter, r *http.Request) {
8282
renderErr(w, err)
8383
return
8484
}
85-
defer conn.Close(r.Context())
85+
defer func() { _ = conn.Close(r.Context()) }()
8686

8787
if err := admin.GrantCreateOnPublic(ctx, dbConn); err != nil {
8888
renderErr(w, err)
@@ -103,7 +103,7 @@ func handleDeleteDatabase(w http.ResponseWriter, r *http.Request) {
103103
renderErr(w, err)
104104
return
105105
}
106-
defer conn.Close(r.Context())
106+
defer func() { _ = conn.Close(r.Context()) }()
107107

108108
err = admin.DeleteDatabase(ctx, conn, name)
109109
if err != nil {

‎internal/api/handle_users.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func handleListUsers(w http.ResponseWriter, r *http.Request) {
1717
renderErr(w, err)
1818
return
1919
}
20-
defer conn.Close(r.Context())
20+
defer func() { _ = conn.Close(r.Context()) }()
2121

2222
users, err := admin.ListUsers(ctx, conn)
2323
if err != nil {
@@ -43,7 +43,7 @@ func handleGetUser(w http.ResponseWriter, r *http.Request) {
4343
renderErr(w, err)
4444
return
4545
}
46-
defer conn.Close(r.Context())
46+
defer func() { _ = conn.Close(r.Context()) }()
4747

4848
user, err := admin.FindUser(ctx, conn, name)
4949
if err != nil {
@@ -64,7 +64,7 @@ func handleCreateUser(w http.ResponseWriter, r *http.Request) {
6464
renderErr(w, err)
6565
return
6666
}
67-
defer conn.Close(r.Context())
67+
defer func() { _ = conn.Close(r.Context()) }()
6868

6969
var input createUserRequest
7070
err = json.NewDecoder(r.Body).Decode(&input)
@@ -112,7 +112,7 @@ func handleDeleteUser(w http.ResponseWriter, r *http.Request) {
112112
renderErr(w, err)
113113
return
114114
}
115-
defer conn.Close(r.Context())
115+
defer func() { _ = conn.Close(r.Context()) }()
116116

117117
databases, err := admin.ListDatabases(ctx, conn)
118118
if err != nil {
@@ -126,7 +126,7 @@ func handleDeleteUser(w http.ResponseWriter, r *http.Request) {
126126
renderErr(w, err)
127127
return
128128
}
129-
defer dbConn.Close(r.Context())
129+
defer func() { _ = dbConn.Close(r.Context()) }()
130130

131131
if err := admin.ReassignOwnership(ctx, dbConn, name, "postgres"); err != nil {
132132
renderErr(w, fmt.Errorf("failed to reassign ownership: %s", err))

‎internal/flycheck/pg.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66

77
"github.com/fly-apps/postgres-flex/internal/flypg"
88
"github.com/jackc/pgx/v5"
9-
"github.com/pkg/errors"
109
"github.com/superfly/fly-checks/check"
1110
)
1211

@@ -17,12 +16,12 @@ const diskCapacityPercentageThreshold = 90.0
1716
func CheckPostgreSQL(ctx context.Context, checks *check.CheckSuite) (*check.CheckSuite, error) {
1817
node, err := flypg.NewNode()
1918
if err != nil {
20-
return checks, errors.Wrap(err, "failed to initialize node")
19+
return checks, fmt.Errorf("failed to initialize node: %s", err)
2120
}
2221

2322
localConn, err := node.NewLocalConnection(ctx, "postgres")
2423
if err != nil {
25-
return checks, errors.Wrap(err, "failed to connect with local node")
24+
return checks, fmt.Errorf("failed to connect with local node: %s", err)
2625
}
2726

2827
repConn, err := node.RepMgr.NewLocalConnection(ctx)
@@ -98,9 +97,8 @@ func connectionCount(ctx context.Context, local *pgx.Conn) (string, error) {
9897
var used, reserved, max int
9998

10099
err := local.QueryRow(ctx, sql).Scan(&used, &reserved, &max)
101-
102100
if err != nil {
103-
return "", fmt.Errorf("%v", err)
101+
return "", fmt.Errorf("failed to query connection count: %s", err)
104102
}
105103

106104
return fmt.Sprintf("%d used, %d reserved, %d max", used, reserved, max), nil

0 commit comments

Comments
 (0)