Keep snapshot polling independent of backup scans (#1437)

Back-port v5 fix 0dca8a037 to v6. Extract pollPVEBackupsAndSnapshots:
the two backup-inventory scans share a bounded backupCtx, which is then
cancelled, and pollGuestSnapshots runs on the parent context. Because
the parent has no deadline, pollGuestSnapshots establishes its own
60s-4min budget instead of inheriting an already-exhausted backup
deadline and skipping entirely (the v6 early-return at
monitor_backups.go made this strictly worse). Adds a regression test
proving snapshots still poll after the storage scan exhausts its budget.
This commit is contained in:
rcourtman
2026-06-04 09:00:30 +01:00
parent a85ec40a4e
commit 5134b39800
2 changed files with 112 additions and 11 deletions
+34 -11
View File
@@ -698,6 +698,39 @@ func (m *Monitor) calculateBackupOperationTimeout(instanceName string) time.Dura
return timeout
}
// pollPVEBackupsAndSnapshots runs the two backup-inventory scans under a shared
// bounded budget, then polls guest snapshots on the parent context so they get
// their own independent budget. A slow storage/backup scan can no longer starve
// snapshot discovery by exhausting the shared timeout before snapshots run.
func (m *Monitor) pollPVEBackupsAndSnapshots(parentCtx context.Context, instanceName string, client PVEClientInterface, nodes []proxmox.Node, nodeEffectiveStatus map[string]string, timeout time.Duration) {
if parentCtx == nil {
parentCtx = context.Background()
}
backupCtx, cancel := context.WithTimeout(parentCtx, timeout)
// Poll backup tasks
m.pollBackupTasks(backupCtx, instanceName, client)
// Poll storage backups - pass nodes to avoid duplicate API calls
m.pollStorageBackupsWithNodes(backupCtx, instanceName, client, nodes, nodeEffectiveStatus)
backupErr := backupCtx.Err()
cancel()
if backupErr != nil && parentCtx.Err() == nil {
log.Warn().
Str("instance", instanceName).
Err(backupErr).
Msg("Backup storage polling budget was exhausted before guest snapshot polling; continuing snapshots with their own bounded poll budget")
}
// Snapshots are independent backup inventory. Passing parentCtx (no deadline)
// lets pollGuestSnapshots establish its own bounded budget instead of
// inheriting an already-exhausted backup deadline and skipping entirely.
m.pollGuestSnapshots(parentCtx, instanceName, client)
}
// pollGuestSnapshots polls snapshots for all VMs and containers
func (m *Monitor) pollGuestSnapshots(ctx context.Context, instanceName string, client PVEClientInterface) {
log.Debug().Str("instance", instanceName).Msg("polling guest snapshots")
@@ -1776,17 +1809,7 @@ func (m *Monitor) pollPVEBackupsAsync(
parentCtx = context.Background()
}
backupCtx, cancel := context.WithTimeout(parentCtx, timeout)
defer cancel()
// Poll backup tasks
m.pollBackupTasks(backupCtx, inst, pveClient)
// Poll storage backups - pass nodes to avoid duplicate API calls
m.pollStorageBackupsWithNodes(backupCtx, inst, pveClient, nodes, nodeEffectiveStatus)
// Poll guest snapshots
m.pollGuestSnapshots(backupCtx, inst, pveClient)
m.pollPVEBackupsAndSnapshots(parentCtx, inst, pveClient, nodes, nodeEffectiveStatus, timeout)
duration := time.Since(startTime)
log.Info().
@@ -55,6 +55,84 @@ func (m *mockPVEClientSnapshots) trackSnapshotConcurrency(ctx context.Context) {
}
}
type backupStorageTimeoutSnapshotClient struct {
mockPVEClientExtra
snapshots []proxmox.Snapshot
snapshotCalls int
storageCalls int
}
func (m *backupStorageTimeoutSnapshotClient) GetBackupTasks(ctx context.Context) ([]proxmox.Task, error) {
return nil, nil
}
func (m *backupStorageTimeoutSnapshotClient) GetStorage(ctx context.Context, node string) ([]proxmox.Storage, error) {
m.storageCalls++
if m.storageCalls > 1 {
return nil, nil
}
<-ctx.Done()
return nil, fmt.Errorf("storage scan exceeded backup inventory budget")
}
func (m *backupStorageTimeoutSnapshotClient) GetVMSnapshots(ctx context.Context, node string, vmid int) ([]proxmox.Snapshot, error) {
m.snapshotCalls++
if err := ctx.Err(); err != nil {
return nil, err
}
return m.snapshots, nil
}
func (m *backupStorageTimeoutSnapshotClient) GetContainerSnapshots(ctx context.Context, node string, vmid int) ([]proxmox.Snapshot, error) {
if err := ctx.Err(); err != nil {
return nil, err
}
return nil, nil
}
func TestMonitor_PollPVEBackupsAndSnapshots_DoesNotStarveSnapshotsAfterStorageTimeout(t *testing.T) {
m := &Monitor{state: models.NewState()}
m.state.UpdateVMsForInstance("pve1", []models.VM{{
ID: "qemu/100",
VMID: 100,
Node: "node1",
Instance: "pve1",
Name: "vm100",
Template: false,
}})
client := &backupStorageTimeoutSnapshotClient{
snapshots: []proxmox.Snapshot{{
Name: "snap_after_storage_timeout",
SnapTime: 4000,
Description: "created while storage scan was slow",
}},
}
m.pollPVEBackupsAndSnapshots(
context.Background(),
"pve1",
client,
[]proxmox.Node{{Node: "node1", Status: "online"}},
map[string]string{"node1": "online"},
time.Millisecond,
)
if client.snapshotCalls == 0 {
t.Fatal("expected guest snapshot polling to run even after storage backup polling exhausted its budget")
}
got := m.state.GetSnapshot().PVEBackups.GuestSnapshots
if len(got) != 1 {
t.Fatalf("expected one guest snapshot after storage timeout, got %#v", got)
}
if got[0].Name != "snap_after_storage_timeout" {
t.Fatalf("expected fresh snapshot after storage timeout, got %#v", got[0])
}
}
func TestMonitor_PollGuestSnapshots_Coverage(t *testing.T) {
m := &Monitor{
state: models.NewState(),