fix: resolve maintenance windows race condition and N+1 query
Some checks failed
CI Pipeline / Rust Format Check (pull_request) Failing after 3s
CI Pipeline / Clippy Lints (pull_request) Failing after 1s
CI Pipeline / Rust Unit Tests (pull_request) Failing after 2s
CI Pipeline / Security Audit (pull_request) Failing after 1s
CI Pipeline / Frontend Lint & Type Check (pull_request) Failing after 4s
CI Pipeline / Build .deb & Release (pull_request) Has been skipped
Some checks failed
CI Pipeline / Rust Format Check (pull_request) Failing after 3s
CI Pipeline / Clippy Lints (pull_request) Failing after 1s
CI Pipeline / Rust Unit Tests (pull_request) Failing after 2s
CI Pipeline / Security Audit (pull_request) Failing after 1s
CI Pipeline / Frontend Lint & Type Check (pull_request) Failing after 4s
CI Pipeline / Build .deb & Release (pull_request) Has been skipped
- Add GET /api/v1/maintenance-windows bulk endpoint to eliminate N+1
per-host API calls (1 request instead of N+1)
- Fix two-phase state update race: setHosts() was called before
setWindowsByHost(), causing React to render hosts with empty windows
- Add AbortController to cancel stale fetch requests on unmount/re-fetch
- Batch state updates atomically (React 18 auto-batching)
- Replace silent catch{} with proper error handling
- Add refreshData() wrapper for mutation handlers and Refresh button
Backend: maintenance_windows.rs - new list_all_windows handler +
all_windows_router(), mounted in main.rs
Frontend: client.ts - new listAll() API method
Frontend: MaintenanceWindowsPage.tsx - rewritten fetchData
This commit is contained in:
@ -288,6 +288,11 @@ pub fn build_router(state: AppState) -> Router {
|
|||||||
"/hosts/{host_id}/maintenance-windows",
|
"/hosts/{host_id}/maintenance-windows",
|
||||||
routes::maintenance_windows::router(),
|
routes::maintenance_windows::router(),
|
||||||
)
|
)
|
||||||
|
// Maintenance windows — bulk list-all endpoint
|
||||||
|
.nest(
|
||||||
|
"/maintenance-windows",
|
||||||
|
routes::maintenance_windows::all_windows_router(),
|
||||||
|
)
|
||||||
// CA root certificate download
|
// CA root certificate download
|
||||||
.nest("/ca", routes::ca::ca_router())
|
.nest("/ca", routes::ca::ca_router())
|
||||||
// Certificate list / renew / revoke
|
// Certificate list / renew / revoke
|
||||||
|
|||||||
36
crates/pm-web/src/routes/maintenance_windows.rs
Executable file → Normal file
36
crates/pm-web/src/routes/maintenance_windows.rs
Executable file → Normal file
@ -1,6 +1,7 @@
|
|||||||
//! Maintenance window management routes.
|
//! Maintenance window management routes.
|
||||||
//!
|
//!
|
||||||
//! GET /api/v1/hosts/{id}/maintenance-windows — list windows for host
|
//! GET /api/v1/hosts/{id}/maintenance-windows — list windows for host
|
||||||
|
//! GET /api/v1/maintenance-windows — list ALL windows (bulk)
|
||||||
//! POST /api/v1/hosts/{id}/maintenance-windows — create window for host
|
//! POST /api/v1/hosts/{id}/maintenance-windows — create window for host
|
||||||
//! PUT /api/v1/hosts/{id}/maintenance-windows/{win_id} — update window
|
//! PUT /api/v1/hosts/{id}/maintenance-windows/{win_id} — update window
|
||||||
//! DELETE /api/v1/hosts/{id}/maintenance-windows/{win_id} — delete window
|
//! DELETE /api/v1/hosts/{id}/maintenance-windows/{win_id} — delete window
|
||||||
@ -32,6 +33,41 @@ pub fn router() -> Router<AppState> {
|
|||||||
.route("/{win_id}", put(update_window).delete(delete_window))
|
.route("/{win_id}", put(update_window).delete(delete_window))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Top-level router for `/api/v1/maintenance-windows` — bulk list-all endpoint.
|
||||||
|
pub fn all_windows_router() -> Router<AppState> {
|
||||||
|
Router::new().route("/", get(list_all_windows))
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── GET /api/v1/maintenance-windows ──────────────────────────────────────────
|
||||||
|
|
||||||
|
/// Bulk endpoint: return every maintenance window across all hosts.
|
||||||
|
/// Eliminates N+1 queries from the frontend (one request instead of one per host).
|
||||||
|
async fn list_all_windows(
|
||||||
|
State(state): State<AppState>,
|
||||||
|
_auth: AuthUser,
|
||||||
|
) -> Result<Json<Value>, (StatusCode, Json<Value>)> {
|
||||||
|
let windows: Vec<MaintenanceWindow> = sqlx::query_as(
|
||||||
|
r#"
|
||||||
|
SELECT id, host_id, label, recurrence, start_at, duration_minutes,
|
||||||
|
recurrence_day, enabled, auto_apply, created_at, updated_at
|
||||||
|
FROM maintenance_windows
|
||||||
|
ORDER BY host_id, created_at ASC
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.fetch_all(&state.db)
|
||||||
|
.await
|
||||||
|
.map_err(|e| {
|
||||||
|
tracing::error!(error = %e, "list_all_windows: query failed");
|
||||||
|
err(
|
||||||
|
StatusCode::INTERNAL_SERVER_ERROR,
|
||||||
|
"internal_error",
|
||||||
|
"Database error",
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
|
||||||
|
Ok(Json(json!({ "windows": windows })))
|
||||||
|
}
|
||||||
|
|
||||||
// ── Error helper ──────────────────────────────────────────────────────────────
|
// ── Error helper ──────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
|
|||||||
@ -5,6 +5,7 @@ import type {
|
|||||||
CreateHostRequest,
|
CreateHostRequest,
|
||||||
CreateJobRequest,
|
CreateJobRequest,
|
||||||
CreateMaintenanceWindowRequest,
|
CreateMaintenanceWindowRequest,
|
||||||
|
MaintenanceWindow,
|
||||||
UpdateMaintenanceWindowRequest,
|
UpdateMaintenanceWindowRequest,
|
||||||
Certificate,
|
Certificate,
|
||||||
IssuedCert,
|
IssuedCert,
|
||||||
@ -176,6 +177,10 @@ export const patchesApi = {
|
|||||||
|
|
||||||
// ── Maintenance Windows API ───────────────────────────────────────────────────
|
// ── Maintenance Windows API ───────────────────────────────────────────────────
|
||||||
export const maintenanceWindowsApi = {
|
export const maintenanceWindowsApi = {
|
||||||
|
/** Bulk: fetch ALL maintenance windows across every host in one request. */
|
||||||
|
listAll: () =>
|
||||||
|
apiClient.get<{ windows: MaintenanceWindow[] }>('/maintenance-windows'),
|
||||||
|
/** Per-host: fetch windows for a single host. */
|
||||||
list: (hostId: string) =>
|
list: (hostId: string) =>
|
||||||
apiClient.get(`/hosts/${hostId}/maintenance-windows`),
|
apiClient.get(`/hosts/${hostId}/maintenance-windows`),
|
||||||
create: (hostId: string, body: CreateMaintenanceWindowRequest) =>
|
create: (hostId: string, body: CreateMaintenanceWindowRequest) =>
|
||||||
|
|||||||
@ -1,4 +1,4 @@
|
|||||||
import { useEffect, useState, useCallback } from 'react'
|
import { useEffect, useState, useCallback, useRef } from 'react'
|
||||||
import {
|
import {
|
||||||
Alert,
|
Alert,
|
||||||
Box,
|
Box,
|
||||||
@ -444,35 +444,70 @@ export default function MaintenanceWindowsPage() {
|
|||||||
const [deleteOpen, setDeleteOpen] = useState(false)
|
const [deleteOpen, setDeleteOpen] = useState(false)
|
||||||
const [deleteWindow, setDeleteWindow] = useState<MaintenanceWindow | null>(null)
|
const [deleteWindow, setDeleteWindow] = useState<MaintenanceWindow | null>(null)
|
||||||
|
|
||||||
// ── Fetch all hosts + their windows ──────────────────────────────────────
|
// ── AbortController ref for cancelling stale fetches ──────────────────────
|
||||||
const fetchData = useCallback(async () => {
|
const abortRef = useRef<AbortController | null>(null)
|
||||||
|
|
||||||
|
// ── Fetch hosts + all maintenance windows in 2 parallel requests ─────────
|
||||||
|
// Uses bulk /maintenance-windows endpoint instead of N+1 per-host calls.
|
||||||
|
// State updates are batched atomically so React never renders hosts without
|
||||||
|
// their windows (the root cause of the "randomly missing data" bug).
|
||||||
|
const fetchData = useCallback(async (signal?: AbortSignal) => {
|
||||||
setLoading(true)
|
setLoading(true)
|
||||||
setError(null)
|
setError(null)
|
||||||
try {
|
try {
|
||||||
const hostsRes = await hostsApi.list({ limit: 500 })
|
// Fetch hosts and ALL windows in parallel — 2 requests, not N+1.
|
||||||
const fetchedHosts: Host[] = hostsRes.data?.hosts ?? hostsRes.data ?? []
|
const [hostsRes, windowsRes] = await Promise.all([
|
||||||
setHosts(fetchedHosts)
|
hostsApi.list({ limit: 500 }),
|
||||||
|
maintenanceWindowsApi.listAll(),
|
||||||
|
])
|
||||||
|
|
||||||
|
// If the request was aborted (e.g. component unmounted or new fetch
|
||||||
|
// started), discard the results silently.
|
||||||
|
if (signal?.aborted) return
|
||||||
|
|
||||||
|
const fetchedHosts: Host[] = hostsRes.data?.hosts ?? hostsRes.data ?? []
|
||||||
|
const allWindows: MaintenanceWindow[] = windowsRes.data?.windows ?? []
|
||||||
|
|
||||||
|
// Group windows by host_id for O(N) lookup.
|
||||||
const windowMap: Record<string, MaintenanceWindow[]> = {}
|
const windowMap: Record<string, MaintenanceWindow[]> = {}
|
||||||
await Promise.all(
|
for (const w of allWindows) {
|
||||||
fetchedHosts.map(async (h) => {
|
if (!windowMap[w.host_id]) windowMap[w.host_id] = []
|
||||||
try {
|
windowMap[w.host_id].push(w)
|
||||||
const res = await maintenanceWindowsApi.list(h.id)
|
}
|
||||||
windowMap[h.id] = res.data?.windows ?? []
|
|
||||||
} catch {
|
// Batch both state updates together — React 18+ auto-batches these
|
||||||
windowMap[h.id] = []
|
// into a single render, eliminating the race condition where hosts
|
||||||
}
|
// rendered with stale/empty windows.
|
||||||
})
|
setHosts(fetchedHosts)
|
||||||
)
|
|
||||||
setWindowsByHost(windowMap)
|
setWindowsByHost(windowMap)
|
||||||
} catch {
|
} catch (err: unknown) {
|
||||||
|
if (signal?.aborted) return // stale request — ignore silently
|
||||||
|
// Only log real errors, not cancellations.
|
||||||
|
if (err instanceof DOMException && err.name === 'AbortError') return
|
||||||
setError('Failed to load hosts or maintenance windows.')
|
setError('Failed to load hosts or maintenance windows.')
|
||||||
} finally {
|
} finally {
|
||||||
setLoading(false)
|
if (!signal?.aborted) {
|
||||||
|
setLoading(false)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
useEffect(() => { fetchData() }, [fetchData])
|
useEffect(() => {
|
||||||
|
// Cancel any in-flight fetch from a previous render.
|
||||||
|
abortRef.current?.abort()
|
||||||
|
const controller = new AbortController()
|
||||||
|
abortRef.current = controller
|
||||||
|
fetchData(controller.signal)
|
||||||
|
return () => { controller.abort() }
|
||||||
|
}, [fetchData])
|
||||||
|
|
||||||
|
// ── Refresh helper: cancels any in-flight fetch, starts a new one ────────
|
||||||
|
const refreshData = useCallback(() => {
|
||||||
|
abortRef.current?.abort()
|
||||||
|
const controller = new AbortController()
|
||||||
|
abortRef.current = controller
|
||||||
|
fetchData(controller.signal)
|
||||||
|
}, [fetchData])
|
||||||
|
|
||||||
// ── Helpers ───────────────────────────────────────────────────────────────
|
// ── Helpers ───────────────────────────────────────────────────────────────
|
||||||
const showSnackbar = (message: string, severity: 'success' | 'error') =>
|
const showSnackbar = (message: string, severity: 'success' | 'error') =>
|
||||||
@ -498,7 +533,7 @@ export default function MaintenanceWindowsPage() {
|
|||||||
})
|
})
|
||||||
setCreateOpen(false)
|
setCreateOpen(false)
|
||||||
showSnackbar('Maintenance window created', 'success')
|
showSnackbar('Maintenance window created', 'success')
|
||||||
await fetchData()
|
refreshData()
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Edit window ───────────────────────────────────────────────────────────
|
// ── Edit window ───────────────────────────────────────────────────────────
|
||||||
@ -529,7 +564,7 @@ export default function MaintenanceWindowsPage() {
|
|||||||
})
|
})
|
||||||
setEditOpen(false)
|
setEditOpen(false)
|
||||||
showSnackbar('Maintenance window updated', 'success')
|
showSnackbar('Maintenance window updated', 'success')
|
||||||
await fetchData()
|
refreshData()
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Delete window ─────────────────────────────────────────────────────────
|
// ── Delete window ─────────────────────────────────────────────────────────
|
||||||
@ -544,7 +579,7 @@ export default function MaintenanceWindowsPage() {
|
|||||||
await maintenanceWindowsApi.remove(deleteWindow.host_id, deleteWindow.id)
|
await maintenanceWindowsApi.remove(deleteWindow.host_id, deleteWindow.id)
|
||||||
setDeleteOpen(false)
|
setDeleteOpen(false)
|
||||||
showSnackbar('Maintenance window deleted', 'success')
|
showSnackbar('Maintenance window deleted', 'success')
|
||||||
await fetchData()
|
refreshData()
|
||||||
} catch {
|
} catch {
|
||||||
showSnackbar('Failed to delete maintenance window', 'error')
|
showSnackbar('Failed to delete maintenance window', 'error')
|
||||||
}
|
}
|
||||||
@ -561,7 +596,7 @@ export default function MaintenanceWindowsPage() {
|
|||||||
</Typography>
|
</Typography>
|
||||||
<Button
|
<Button
|
||||||
startIcon={<RefreshIcon />}
|
startIcon={<RefreshIcon />}
|
||||||
onClick={fetchData}
|
onClick={refreshData}
|
||||||
disabled={loading}
|
disabled={loading}
|
||||||
>
|
>
|
||||||
Refresh
|
Refresh
|
||||||
|
|||||||
Reference in New Issue
Block a user