Skip to content

Commit f8e6e7c

Browse files
committed
Merge branch 'fix/create-clone-poller-leak' into 'master'
fix(ui): stop clone status polling on page unmount Closes #743 See merge request postgres-ai/database-lab!1172
2 parents 2ce1a2a + bd574c7 commit f8e6e7c

6 files changed

Lines changed: 174 additions & 16 deletions

File tree

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
2+
3+
import {
4+
ClonePoller,
5+
UNSTABLE_CLONE_UPDATE_TIMEOUT,
6+
} from '@postgres.ai/shared/utils/clonePoller'
7+
8+
describe('ClonePoller', () => {
9+
beforeEach(() => vi.useFakeTimers())
10+
afterEach(() => vi.useRealTimers())
11+
12+
it('is running by default', () => {
13+
expect(new ClonePoller().isStopped).toBe(false)
14+
})
15+
16+
it('runs the scheduled tick after the default interval', () => {
17+
const poller = new ClonePoller()
18+
const tick = vi.fn()
19+
20+
poller.scheduleNext(tick)
21+
vi.advanceTimersByTime(UNSTABLE_CLONE_UPDATE_TIMEOUT - 1)
22+
expect(tick).not.toHaveBeenCalled()
23+
24+
vi.advanceTimersByTime(1)
25+
expect(tick).toHaveBeenCalledTimes(1)
26+
})
27+
28+
it('honors a custom interval', () => {
29+
const poller = new ClonePoller(50)
30+
const tick = vi.fn()
31+
32+
poller.scheduleNext(tick)
33+
vi.advanceTimersByTime(50)
34+
expect(tick).toHaveBeenCalledTimes(1)
35+
})
36+
37+
it('keeps only the latest pending tick', () => {
38+
const poller = new ClonePoller(100)
39+
const first = vi.fn()
40+
const second = vi.fn()
41+
42+
poller.scheduleNext(first)
43+
poller.scheduleNext(second)
44+
vi.advanceTimersByTime(100)
45+
46+
expect(first).not.toHaveBeenCalled()
47+
expect(second).toHaveBeenCalledTimes(1)
48+
})
49+
50+
it('stop() cancels a pending tick and marks the poller stopped', () => {
51+
const poller = new ClonePoller(100)
52+
const tick = vi.fn()
53+
54+
poller.scheduleNext(tick)
55+
poller.stop()
56+
vi.advanceTimersByTime(100)
57+
58+
expect(tick).not.toHaveBeenCalled()
59+
expect(poller.isStopped).toBe(true)
60+
})
61+
62+
it('scheduleNext() is a no-op once stopped', () => {
63+
const poller = new ClonePoller(100)
64+
const tick = vi.fn()
65+
66+
poller.stop()
67+
poller.scheduleNext(tick)
68+
vi.advanceTimersByTime(100)
69+
70+
expect(tick).not.toHaveBeenCalled()
71+
})
72+
73+
it('start() re-arms a stopped poller', () => {
74+
const poller = new ClonePoller(100)
75+
const tick = vi.fn()
76+
77+
poller.stop()
78+
poller.start()
79+
expect(poller.isStopped).toBe(false)
80+
81+
poller.scheduleNext(tick)
82+
vi.advanceTimersByTime(100)
83+
expect(tick).toHaveBeenCalledTimes(1)
84+
})
85+
86+
it('cancel() drops the pending tick but keeps scheduling enabled', () => {
87+
const poller = new ClonePoller(100)
88+
const dropped = vi.fn()
89+
const next = vi.fn()
90+
91+
poller.scheduleNext(dropped)
92+
poller.cancel()
93+
vi.advanceTimersByTime(100)
94+
expect(dropped).not.toHaveBeenCalled()
95+
expect(poller.isStopped).toBe(false)
96+
97+
poller.scheduleNext(next)
98+
vi.advanceTimersByTime(100)
99+
expect(next).toHaveBeenCalledTimes(1)
100+
})
101+
})

ui/packages/shared/pages/Clone/index.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ export const Clone = observer((props: Props) => {
205205
stores.main.load(props.instanceId, props.cloneId)
206206
}, [])
207207

208+
// Stop clone status polling on unmount.
209+
useEffect(() => () => stores.main.stopPolling(), [])
210+
208211
const {
209212
instance,
210213
snapshots,

ui/packages/shared/pages/Clone/stores/Main.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,10 @@ import {
1919
import { Clone } from '@postgres.ai/shared/types/api/entities/clone'
2020
import { Instance } from '@postgres.ai/shared/types/api/entities/instance'
2121
import { checkIsCloneStable } from '@postgres.ai/shared/utils/clone'
22+
import { ClonePoller } from '@postgres.ai/shared/utils/clonePoller'
2223
import { getTextFromUnknownApiError } from '@postgres.ai/shared/utils/api'
2324
import { InitWS } from '@postgres.ai/shared/types/api/endpoints/initWS'
2425

25-
const UNSTABLE_CLONE_UPDATE_TIMEOUT = 1000
26-
2726
export type Api = SnapshotsApi & {
2827
getInstance: GetInstance
2928
getClone: GetClone
@@ -58,7 +57,7 @@ export class MainStore {
5857

5958
isReloading = false
6059

61-
private cloneUpdateTimeout?: number
60+
private readonly poller = new ClonePoller()
6261

6362
private readonly api: Api
6463

@@ -75,6 +74,8 @@ export class MainStore {
7574
}
7675

7776
load = async (instanceId: string, cloneId: string) => {
77+
this.poller.start()
78+
7879
const [isInstanceOk, isCloneOk, isSnapshotsLoaded] = await Promise.all([
7980
this.loadInstance(instanceId),
8081
this.loadClone(instanceId, cloneId),
@@ -92,6 +93,8 @@ export class MainStore {
9293
return isSuccess
9394
}
9495

96+
stopPolling = () => this.poller.stop()
97+
9598
private loadInstance = async (instanceId: string) => {
9699
const { response, error } = await this.api.getInstance({
97100
instanceId,
@@ -116,18 +119,17 @@ export class MainStore {
116119
}
117120

118121
private loadClone = async (instanceId: string, cloneId: string) => {
119-
window.clearTimeout(this.cloneUpdateTimeout)
122+
if (this.poller.isStopped) return false
123+
124+
this.poller.cancel()
120125

121126
const { response, error } = await this.api.getClone({ instanceId, cloneId })
122127

123128
if (response) {
124129
this.clone = response
125130

126131
if (!this.isCloneStable)
127-
this.cloneUpdateTimeout = window.setTimeout(
128-
() => this.loadClone(instanceId, cloneId),
129-
UNSTABLE_CLONE_UPDATE_TIMEOUT,
130-
)
132+
this.poller.scheduleNext(() => this.loadClone(instanceId, cloneId))
131133
}
132134

133135
if (error) {

ui/packages/shared/pages/CreateClone/index.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ export const CreateClone = observer((props: Props) => {
166166
history.push(props.routes.clone(stores.main.clone.id))
167167
}, [stores.main.clone, stores.main.isCloneStable])
168168

169+
// Stop clone status polling on unmount.
170+
useEffect(() => () => stores.main.stopPolling(), [])
171+
169172
const headRendered = (
170173
<>
171174
{/* //TODO: make global reset styles. */}

ui/packages/shared/pages/CreateClone/stores/Main.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,11 @@ import {
1313
} from '@postgres.ai/shared/stores/Snapshots'
1414
import { getTextFromUnknownApiError } from '@postgres.ai/shared/utils/api'
1515
import { checkIsCloneStable } from '@postgres.ai/shared/utils/clone'
16+
import { ClonePoller } from '@postgres.ai/shared/utils/clonePoller'
1617

1718
import { FormValues } from '../useForm'
1819
import { InitWS } from '@postgres.ai/shared/types/api/endpoints/initWS'
1920

20-
const UNSTABLE_CLONE_UPDATE_TIMEOUT = 1000
21-
2221
export type MainStoreApi = SnapshotsApi & {
2322
getInstance: GetInstance
2423
createClone: CreateClone
@@ -37,7 +36,7 @@ export class MainStore {
3736
clone: Clone | null = null
3837
cloneError: string | null = null
3938

40-
private cloneUpdateTimeout?: number
39+
private readonly poller = new ClonePoller()
4140

4241
private readonly api: MainStoreApi
4342

@@ -73,6 +72,8 @@ export class MainStore {
7372
createClone = async (data: FormValues) => {
7473
if (!this.instance) return false
7574

75+
this.poller.start()
76+
7677
const { response, error } = await this.api.createClone({
7778
...data,
7879
instanceId: this.instance.id,
@@ -115,22 +116,23 @@ export class MainStore {
115116
return response
116117
}
117118

119+
stopPolling = () => this.poller.stop()
120+
118121
private updateCloneUntilStable = async (args: {
119122
instanceId: string
120123
cloneId: string
121124
}) => {
122-
window.clearTimeout(this.cloneUpdateTimeout)
125+
if (this.poller.isStopped) return
126+
127+
this.poller.cancel()
123128

124129
const { response, error } = await this.api.getClone(args)
125130

126131
if (response) {
127132
this.clone = response
128133

129134
if (!this.isCloneStable)
130-
this.cloneUpdateTimeout = window.setTimeout(
131-
() => this.updateCloneUntilStable(args),
132-
UNSTABLE_CLONE_UPDATE_TIMEOUT,
133-
)
135+
this.poller.scheduleNext(() => this.updateCloneUntilStable(args))
134136
}
135137

136138
if (error)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*--------------------------------------------------------------------------
2+
* Copyright (c) 2019-2021, Postgres.ai, Nikolay Samokhvalov nik@postgres.ai
3+
* All Rights Reserved. Proprietary and confidential.
4+
* Unauthorized copying of this file, via any medium is strictly prohibited
5+
*--------------------------------------------------------------------------
6+
*/
7+
8+
// interval between clone status polls while a clone is still unstable.
9+
export const UNSTABLE_CLONE_UPDATE_TIMEOUT = 1000
10+
11+
// ClonePoller owns a single self-rescheduling timeout used to poll clone status
12+
// until it becomes stable. It is reusable: start() re-arms a poller that was
13+
// previously stopped, and stop() cancels any pending tick and blocks further
14+
// scheduling — including a tick that an in-flight request is about to queue.
15+
export class ClonePoller {
16+
private timeout?: ReturnType<typeof setTimeout>
17+
private stopped = false
18+
19+
constructor(private readonly intervalMs: number = UNSTABLE_CLONE_UPDATE_TIMEOUT) {}
20+
21+
get isStopped() {
22+
return this.stopped
23+
}
24+
25+
// start re-arms the poller so a reused store polls again.
26+
start = () => {
27+
this.stopped = false
28+
}
29+
30+
// stop halts polling and cancels any pending tick.
31+
stop = () => {
32+
this.stopped = true
33+
clearTimeout(this.timeout)
34+
}
35+
36+
// cancel drops a pending tick without blocking future scheduling.
37+
cancel = () => {
38+
clearTimeout(this.timeout)
39+
}
40+
41+
// scheduleNext queues the next tick, unless the poller has been stopped.
42+
scheduleNext = (tick: () => void) => {
43+
clearTimeout(this.timeout)
44+
if (this.stopped) return
45+
this.timeout = setTimeout(tick, this.intervalMs)
46+
}
47+
}

0 commit comments

Comments
 (0)