Skip to content

Commit

Permalink
rework logic, use set diff instead of !setEq to trigger clock restart
Browse files Browse the repository at this point in the history
  • Loading branch information
david-crespo committed Aug 23, 2024
1 parent 1eb7244 commit 7dbe8c6
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 12 deletions.
29 changes: 18 additions & 11 deletions app/pages/project/instances/InstancesPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { CreateLink } from '~/ui/lib/CreateButton'
import { EmptyMessage } from '~/ui/lib/EmptyMessage'
import { PageHeader, PageTitle } from '~/ui/lib/PageHeader'
import { TableActions } from '~/ui/lib/Table'
import { isSetEqual } from '~/util/array'
import { setDiff } from '~/util/array'
import { docLinks } from '~/util/links'
import { pb } from '~/util/path-builder'

Expand Down Expand Up @@ -108,18 +108,25 @@ export function InstancesPage() {
// always update. we don't have to worry about doing this in all the branches below.
transitioningInstances.current = nextTransitioning

// if no instances are transitioning, do not poll
if (nextTransitioning.size === 0) return false

// if the set of transitioning instances hasn't changed, we only poll if we haven't hit the timeout
if (isSetEqual(prevTransitioning, nextTransitioning)) {
const elapsed = Date.now() - pollingStartTime.current
return elapsed < POLL_TIMEOUT ? POLL_INTERVAL : false
// We use this setDiff logic instead of just checking whether
// the set has changed because if you have two transitioning
// instances and one stops transitioning, then that's a change in
// the set, but you shouldn't start polling because of it! What
// you want to look for is new transitioning instances, i.e.,
// nextTransitioning.difference(prevTransitioning).size > 0
const anyTransitioning = nextTransitioning.size > 0
const anyNewTransitioning = setDiff(nextTransitioning, prevTransitioning).size > 0

// if we have new transitioning instances, restart the timeout clock
if (anyNewTransitioning) pollingStartTime.current = Date.now()

// important that elapsed is calculated *after* bumping start time
const elapsed = Date.now() - pollingStartTime.current
if (anyTransitioning && elapsed < POLL_TIMEOUT) {
return POLL_INTERVAL
}

// if we have new transitioning instances, always poll and restart the window
pollingStartTime.current = Date.now()
return POLL_INTERVAL
return false
},
}
)
Expand Down
11 changes: 10 additions & 1 deletion app/util/array.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { type ReactElement } from 'react'
import { expect, test } from 'vitest'

import { groupBy, intersperse, isSetEqual } from './array'
import { groupBy, intersperse, isSetEqual, setDiff } from './array'

test('groupBy', () => {
expect(
Expand Down Expand Up @@ -72,3 +72,12 @@ test('isSetEqual', () => {

expect(isSetEqual(new Set([{}]), new Set([{}]))).toBe(false)
})

test.only('setDiff', () => {
expect(setDiff(new Set(), new Set())).toEqual(new Set())
expect(setDiff(new Set(['a']), new Set())).toEqual(new Set(['a']))
expect(setDiff(new Set(), new Set(['a']))).toEqual(new Set())
expect(setDiff(new Set(['b', 'a', 'c']), new Set(['b', 'd']))).toEqual(
new Set(['a', 'c'])
)
})
5 changes: 5 additions & 0 deletions app/util/array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,8 @@ export function isSetEqual<T>(a: Set<T>, b: Set<T>): boolean {
}
return true
}

/** Set `a - b` */
export function setDiff<T>(a: Set<T>, b: Set<T>): Set<T> {
return new Set([...a].filter((x) => !b.has(x)))
}

0 comments on commit 7dbe8c6

Please sign in to comment.