Polling on instance list (#2391)

* first pass at instance list polling

* do it cleaner

* add instance state to key

* poll for 30 seconds on a 3 second interval

* rework logic, use set diff instead of !setEq to trigger clock restart

* add updated time. poll fast vs. slow, not fast vs. not at all

* test polling in e2es

* forgot that we only want to bother polling fast if there are any instances transitioning

* comment tweak

* fix typo in test. guitly

* stray test.only

Author: david-crespo

app/pages/project/instances/InstancesPage.tsx

@@ -7,12 +7,13 @@
  */
 import { createColumnHelper } from '@tanstack/react-table'
 import { filesize } from 'filesize'
-import { useMemo } from 'react'
+import { useMemo, useRef } from 'react'
 import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom'
 
 import { apiQueryClient, usePrefetchedApiQuery, type Instance } from '@oxide/api'
 import { Instances24Icon } from '@oxide/design-system/icons/react'
 
+import { instanceTransitioning } from '~/api/util'
 import { InstanceDocsPopover } from '~/components/InstanceDocsPopover'
 import { RefreshButton } from '~/components/RefreshButton'
 import { getProjectSelector, useProjectSelector, useQuickActions } from '~/hooks'
@@ -25,6 +26,9 @@ 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 { Tooltip } from '~/ui/lib/Tooltip'
+import { setDiff } from '~/util/array'
+import { toLocaleTimeString } from '~/util/date'
 import { pb } from '~/util/path-builder'
 
 import { useMakeInstanceActions } from './actions'
@@ -51,6 +55,12 @@ InstancesPage.loader = async ({ params }: LoaderFunctionArgs) => {
 
 const refetchInstances = () => apiQueryClient.invalidateQueries('instanceList')
 
+const sec = 1000 // ms, obviously
+const POLL_FAST_TIMEOUT = 30 * sec
+// a little slower than instance detail because this is a bigger response
+const POLL_INTERVAL_FAST = 3 * sec
+const POLL_INTERVAL_SLOW = 60 * sec
+
 export function InstancesPage() {
   const { project } = useProjectSelector()
 
@@ -59,9 +69,61 @@ export function InstancesPage() {
     { onSuccess: refetchInstances, onDelete: refetchInstances }
   )
 
-  const { data: instances } = usePrefetchedApiQuery('instanceList', {
-    query: { project, limit: PAGE_SIZE },
-  })
+  // this is a whole thing. sit down.
+
+  // We initialize this set as empty because we don't have the instances on hand
+  // yet. This is fine because the first fetch will recognize the presence of
+  // any transitioning instances as a change in state and initiate polling
+  const transitioningInstances = useRef<Set<string>>(new Set())
+  const pollingStartTime = useRef<number>(Date.now())
+
+  const { data: instances, dataUpdatedAt } = usePrefetchedApiQuery(
+    'instanceList',
+    { query: { project, limit: PAGE_SIZE } },
+    {
+      // The point of all this is to poll quickly for a certain amount of time
+      // after some instance in the current page enters a transitional state
+      // like starting or stopping. After that, it will keep polling, but more
+      // slowly. For example, if you stop an instance, its state will change to
+      // `stopping`, which will cause this logic to start polling the list until
+      // it lands in `stopped`, at which point it will poll only slowly because
+      // `stopped` is not considered transitional.
+      refetchInterval({ state: { data } }) {
+        const prevTransitioning = transitioningInstances.current
+        const nextTransitioning = new Set(
+          // Data will never actually be undefined because of the prefetch but whatever
+          (data?.items || [])
+            .filter(instanceTransitioning)
+            // These are strings of instance ID + current state. This is done because
+            // of the case where an instance is stuck in starting (for example), polling
+            // times out, and then you manually stop it. Without putting the state in the
+            // the key, that stop action would not be registered as a change in the set
+            // of transitioning instances.
+            .map((i) => i.id + '|' + i.runState)
+        )
+
+        // always update the ledger to the current state
+        transitioningInstances.current = nextTransitioning
+
+        // We use this set difference logic instead of set equality 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
+        // fast because of it! What you want to look for is *new* transitioning
+        // instances.
+        const anyTransitioning = nextTransitioning.size > 0
+        const anyNewTransitioning = setDiff(nextTransitioning, prevTransitioning).size > 0
+
+        // if there are new instances in transitioning, restart the timeout window
+        if (anyNewTransitioning) pollingStartTime.current = Date.now()
+
+        // important that elapsed is calculated *after* potentially bumping start time
+        const elapsed = Date.now() - pollingStartTime.current
+        return anyTransitioning && elapsed < POLL_FAST_TIMEOUT
+          ? POLL_INTERVAL_FAST
+          : POLL_INTERVAL_SLOW
+      },
+    }
+  )
 
   const navigate = useNavigate()
   useQuickActions(
@@ -132,8 +194,20 @@ export function InstancesPage() {
         <PageTitle icon={<Instances24Icon />}>Instances</PageTitle>
         <InstanceDocsPopover />
       </PageHeader>
-      <TableActions>
-        <RefreshButton onClick={refetchInstances} />
+      {/* Avoid changing justify-end on TableActions for this one case. We can
+       * fix this properly when we add refresh and filtering for all tables. */}
+      <TableActions className="!-mt-6 !justify-between">
+        <div className="flex items-center gap-2">
+          <RefreshButton onClick={refetchInstances} />
+          <Tooltip
+            content="Auto-refresh is more frequent after instance actions"
+            delay={150}
+          >
+            <span className="text-sans-sm text-tertiary">
+              Updated {toLocaleTimeString(new Date(dataUpdatedAt))}
+            </span>
+          </Tooltip>
+        </div>
         <CreateLink to={pb.instancesNew({ project })}>New Instance</CreateLink>
       </TableActions>
       <Table columns={columns} emptyState={<EmptyState />} />

app/pages/project/instances/instance/InstancePage.tsx

@@ -86,6 +86,8 @@ InstancePage.loader = async ({ params }: LoaderFunctionArgs) => {
   return null
 }
 
+const POLL_INTERVAL = 1000
+
 export function InstancePage() {
   const instanceSelector = useInstanceSelector()
 
@@ -107,7 +109,7 @@ export function InstancePage() {
     },
     {
       refetchInterval: ({ state: { data: instance } }) =>
-        instance && instanceTransitioning(instance) ? 1000 : false,
+        instance && instanceTransitioning(instance) ? POLL_INTERVAL : false,
     }
   )
 
@@ -169,7 +171,7 @@ export function InstancePage() {
             <div className="flex">
               <InstanceStatusBadge status={instance.runState} />
               {polling && (
-                <Tooltip content="Auto-refreshing while state changes" delay={150}>
+                <Tooltip content="Auto-refreshing while status changes" delay={150}>
                   <button type="button">
                     <Spinner className="ml-2" />
                   </button>

app/ui/lib/Table.tsx

@@ -115,7 +115,7 @@ Table.Cell = ({ height = 'small', className, children, ...props }: TableCellProp
  * Used _outside_ of the `Table`, this element wraps buttons that sit on top
  * of the table.
  */
-export const TableActions = classed.div`-mt-11 mb-3 flex justify-end space-x-2`
+export const TableActions = classed.div`-mt-11 mb-3 flex justify-end gap-2`
 
 export const TableEmptyBox = classed.div`flex h-full max-h-[480px] items-center justify-center rounded-lg border border-secondary p-4`
 

app/util/array.spec.tsx

@@ -8,7 +8,7 @@
 import { type ReactElement } from 'react'
 import { expect, test } from 'vitest'
 
-import { groupBy, intersperse } from './array'
+import { groupBy, intersperse, isSetEqual, setDiff } from './array'
 
 test('groupBy', () => {
   expect(
@@ -61,3 +61,23 @@ test('intersperse', () => {
   expect(result.map(getText)).toEqual(['a', ',', 'b', ',', 'or', 'c'])
   expect(result.map(getKey)).toEqual(['a', 'sep-1', 'b', 'sep-2', 'conj', 'c'])
 })
+
+test('isSetEqual', () => {
+  expect(isSetEqual(new Set(), new Set())).toBe(true)
+  expect(isSetEqual(new Set(['a', 'b', 'c']), new Set(['a', 'b', 'c']))).toBe(true)
+
+  expect(isSetEqual(new Set(['a']), new Set(['b']))).toBe(false)
+  expect(isSetEqual(new Set(['a']), new Set(['a', 'b']))).toBe(false)
+  expect(isSetEqual(new Set(['a', 'b']), new Set(['a']))).toBe(false)
+
+  expect(isSetEqual(new Set([{}]), new Set([{}]))).toBe(false)
+})
+
+test('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'])
+  )
+})

app/util/array.ts

@@ -38,3 +38,18 @@ export function intersperse(
     return [sep0, item]
   })
 }
+
+export function isSetEqual<T>(a: Set<T>, b: Set<T>): boolean {
+  if (a.size !== b.size) return false
+  for (const item of a) {
+    if (!b.has(item)) {
+      return false
+    }
+  }
+  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)))
+}

test/e2e/instance.e2e.ts

@@ -5,7 +5,7 @@
  *
  * Copyright Oxide Computer Company
  */
-import { expect, expectRowVisible, refreshInstance, sleep, test } from './utils'
+import { clickRowAction, expect, expectRowVisible, test } from './utils'
 
 test('can delete a failed instance', async ({ page }) => {
   await page.goto('/projects/mock-project/instances')
@@ -26,45 +26,66 @@ test('can delete a failed instance', async ({ page }) => {
 test('can stop and delete a running instance', async ({ page }) => {
   await page.goto('/projects/mock-project/instances')
 
+  const table = page.getByRole('table')
+  await expectRowVisible(table, {
+    name: 'db1',
+    status: expect.stringContaining('running'),
+  })
   const row = page.getByRole('row', { name: 'db1', exact: false })
-  await expect(row).toBeVisible()
-  await expect(row.getByRole('cell', { name: /running/ })).toBeVisible()
 
   // can't delete, can stop
   await row.getByRole('button', { name: 'Row actions' }).click()
   await expect(page.getByRole('menuitem', { name: 'Delete' })).toBeDisabled()
   await page.getByRole('menuitem', { name: 'Stop' }).click()
   await page.getByRole('button', { name: 'Confirm' }).click()
 
-  await sleep(4000)
-  await refreshInstance(page)
-
-  // now it's stopped
-  await expect(row.getByRole('cell', { name: /stopped/ })).toBeVisible()
+  // polling makes it go stopping and then stopped
+  await expectRowVisible(table, {
+    name: 'db1',
+    status: expect.stringContaining('stopping'),
+  })
+  await expectRowVisible(table, {
+    name: 'db1',
+    status: expect.stringContaining('stopped'),
+  })
 
   // now delete
-  await row.getByRole('button', { name: 'Row actions' }).click()
-  await page.getByRole('menuitem', { name: 'Delete' }).click()
+  await clickRowAction(page, 'db1', 'Delete')
   await page.getByRole('button', { name: 'Confirm' }).click()
 
   await expect(row).toBeHidden() // bye
 })
 
-test('can stop a starting instance', async ({ page }) => {
+test('can stop a starting instance, then start it again', async ({ page }) => {
   await page.goto('/projects/mock-project/instances')
 
-  const row = page.getByRole('row', { name: 'not-there-yet', exact: false })
-  await expect(row).toBeVisible()
-  await expect(row.getByRole('cell', { name: /starting/ })).toBeVisible()
+  const table = page.getByRole('table')
+  await expectRowVisible(table, {
+    name: 'not-there-yet',
+    status: expect.stringContaining('starting'),
+  })
 
-  await row.getByRole('button', { name: 'Row actions' }).click()
-  await page.getByRole('menuitem', { name: 'Stop' }).click()
+  await clickRowAction(page, 'not-there-yet', 'Stop')
   await page.getByRole('button', { name: 'Confirm' }).click()
 
-  await sleep(4000)
-  await refreshInstance(page)
+  await expectRowVisible(table, {
+    name: 'not-there-yet',
+    status: expect.stringContaining('stopping'),
+  })
+  await expectRowVisible(table, {
+    name: 'not-there-yet',
+    status: expect.stringContaining('stopped'),
+  })
 
-  await expect(row.getByRole('cell', { name: /stopped/ })).toBeVisible()
+  await clickRowAction(page, 'not-there-yet', 'Start')
+  await expectRowVisible(table, {
+    name: 'not-there-yet',
+    status: expect.stringContaining('starting'),
+  })
+  await expectRowVisible(table, {
+    name: 'not-there-yet',
+    status: expect.stringContaining('running'),
+  })
 })
 
 test('delete from instance detail', async ({ page }) => {

test/e2e/utils.ts

@@ -111,15 +111,10 @@ export async function stopInstance(page: Page) {
   await page.getByRole('menuitem', { name: 'Stop' }).click()
   await page.getByRole('button', { name: 'Confirm' }).click()
   await closeToast(page)
-  await sleep(2000)
-  await refreshInstance(page)
+  // don't need to manually refresh because of polling
   await expect(page.getByText('statusstopped')).toBeVisible()
 }
 
-export async function refreshInstance(page: Page) {
-  await page.getByRole('button', { name: 'Refresh data' }).click()
-}
-
 /**
  * Close toast and wait for it to fade out. For some reason it prevents things
  * from working, but only in tests as far as we can tell.