forked from Disciplr-Org/Disciplr-backend
-
Notifications
You must be signed in to change notification settings - Fork 0
Expand file tree
/
Copy pathissue-1061-close-pg-pool.patch
More file actions
165 lines (161 loc) · 5.89 KB
/
Copy pathissue-1061-close-pg-pool.patch
File metadata and controls
165 lines (161 loc) · 5.89 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
diff --git a/src/db/pool.ts b/src/db/pool.ts
index cea8ef3..dd59bda 100644
--- a/src/db/pool.ts
+++ b/src/db/pool.ts
@@ -19,3 +19,25 @@ export const getPgPool = (): Pool | null => {
return null
}
}
+
+/**
+ * Issue #1061: closes the lazily-created pool (if one was ever created) and
+ * clears the module-level cache so a subsequent getPgPool() call creates a
+ * fresh pool rather than reusing ended connections.
+ *
+ * Called from the graceful-shutdown handler (see server/shutdown.ts) so
+ * in-flight and idle Postgres connections are released on SIGTERM/SIGINT
+ * instead of being left open until the OS tears them down, which can
+ * exhaust the database's max_connections during rolling deploys.
+ *
+ * Safe to call even if no pool was ever created (e.g. DATABASE_URL unset) —
+ * this is a no-op in that case rather than throwing.
+ */
+export const closePgPool = async (): Promise<void> => {
+ if (!pool) {
+ return
+ }
+ const current = pool
+ pool = null
+ await current.end()
+}
diff --git a/src/index.ts b/src/index.ts
index fa183af..a63a0ef 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -22,7 +22,8 @@ import {
securityMetricsMiddleware,
securityRateLimitMiddleware,
} from "./security/abuse-monitor.js";
-import { initializeDatabase, closeDatabase } from "./db/database.js";
+import { initializeDatabase } from "./db/database.js";
+import { closePgPool } from "./db/pool.js";
import { getEtlWorker } from "./services/etlWorker.js";
import { createShutdownHandler } from "./server/shutdown.js";
import { createNotificationService } from "./services/notifications/factory.js";
@@ -87,7 +88,7 @@ const shutdownHandler = createShutdownHandler({
server,
jobSystem,
etlWorker,
- closeDb: closeDatabase,
+ closeDb: closePgPool,
});
for (const signal of ["SIGINT", "SIGTERM"] as const) {
diff --git a/src/server/shutdown.ts b/src/server/shutdown.ts
index 71307a1..8694187 100644
--- a/src/server/shutdown.ts
+++ b/src/server/shutdown.ts
@@ -12,7 +12,7 @@ export interface ShutdownOptions {
server: Server;
jobSystem: BackgroundJobSystem;
etlWorker: ETLWorker;
- closeDb: () => void;
+ closeDb: () => void | Promise<void>;
}
/**
@@ -119,7 +119,7 @@ export function createShutdownHandler(options: ShutdownOptions) {
// 4. Close Database
console.log("[Shutdown] Closing database connection...");
- closeDb();
+ await closeDb();
console.log("[Shutdown] Graceful shutdown completed successfully");
process.exit(0);
diff --git a/src/tests/pool.test.ts b/src/tests/pool.test.ts
new file mode 100644
index 0000000..34fd371
--- /dev/null
+++ b/src/tests/pool.test.ts
@@ -0,0 +1,84 @@
+import { jest, describe, it, expect, beforeAll, beforeEach } from '@jest/globals'
+
+// ─── Mocks ────────────────────────────────────────────────────────────────────
+//
+// Real `pg.Pool` opens a TCP connection on construction, so it's mocked here
+// the same way the rest of the suite mocks it (see jobs.overlapGuard.test.ts)
+// — a fake constructor that records instances so tests can assert on
+// `.end()` without touching a real Postgres server.
+
+const poolInstances: Array<{ end: jest.Mock<() => Promise<void>> }> = []
+
+jest.unstable_mockModule('pg', () => {
+ class MockPool {
+ end = jest.fn<() => Promise<void>>().mockResolvedValue(undefined)
+ constructor(public options: unknown) {
+ poolInstances.push(this)
+ }
+ }
+ return { Pool: MockPool }
+})
+
+let mockDatabaseUrl: string | undefined = 'postgres://test:test@localhost:5432/test'
+
+jest.unstable_mockModule('../config/index.js', () => ({
+ getEnv: () => ({ DATABASE_URL: mockDatabaseUrl }),
+}))
+
+// ─── Import subject ──────────────────────────────────────────────────────────
+//
+// Dynamically imported in beforeAll (after mocks are registered), matching
+// the pattern used in openapi.contract.test.ts. The module-level `pool`
+// singleton is reset between tests via closePgPool() in beforeEach rather
+// than jest.resetModules(), since resetModules() would drop the pg/config
+// mock registrations for any subsequently re-imported module.
+
+let getPgPool: typeof import('../db/pool.js').getPgPool
+let closePgPool: typeof import('../db/pool.js').closePgPool
+
+beforeAll(async () => {
+ ;({ getPgPool, closePgPool } = await import('../db/pool.js'))
+})
+
+describe('db/pool — closePgPool', () => {
+ beforeEach(async () => {
+ // Leave no cached pool behind from a prior test.
+ await closePgPool()
+ poolInstances.length = 0
+ mockDatabaseUrl = 'postgres://test:test@localhost:5432/test'
+ })
+
+ it('is a no-op when no pool was ever created', async () => {
+ await expect(closePgPool()).resolves.toBeUndefined()
+ expect(poolInstances).toHaveLength(0)
+ })
+
+ it('is a no-op when DATABASE_URL was never set', async () => {
+ mockDatabaseUrl = undefined
+ expect(getPgPool()).toBeNull()
+ await expect(closePgPool()).resolves.toBeUndefined()
+ expect(poolInstances).toHaveLength(0)
+ })
+
+ it('ends the cached pool and clears the cache so a later call creates a fresh one', async () => {
+ const first = getPgPool()
+ expect(first).not.toBeNull()
+ expect(poolInstances).toHaveLength(1)
+
+ await closePgPool()
+ expect(poolInstances[0]!.end).toHaveBeenCalledTimes(1)
+
+ const second = getPgPool()
+ expect(poolInstances).toHaveLength(2)
+ expect(second).not.toBe(first)
+ })
+
+ it('is safe to call more than once in a row', async () => {
+ getPgPool()
+ await closePgPool()
+ await expect(closePgPool()).resolves.toBeUndefined()
+
+ // Only the single real pool was ever ended — the second call was a no-op.
+ expect(poolInstances[0]!.end).toHaveBeenCalledTimes(1)
+ })
+})