Skip to content

perf: batch volume existence checks in delete-snapshots#35

Merged
dannysteenman merged 2 commits intomainfrom
devin/1771790520-batch-volume-exists
Feb 23, 2026
Merged

perf: batch volume existence checks in delete-snapshots#35
dannysteenman merged 2 commits intomainfrom
devin/1771790520-batch-volume-exists

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Feb 22, 2026

perf: batch volume existence checks in delete-snapshots

Summary

Replaces the N+1 DescribeVolumes API call pattern in runDeleteSnapshots with a batch lookup. Previously, each orphaned snapshot triggered an individual DescribeVolumes call to check if its source volume still exists. Now, all candidate volume IDs are collected upfront and checked in a single batch call (up to 200 IDs per request).

When a batch call fails with InvalidVolume.NotFound (meaning at least one volume in the batch is gone), the function falls back to individual lookups for that batch only.

The old volumeExists function is replaced by listExistingVolumeIDs.

Review & Testing Checklist for Human

  • Duplicated filtering logic: The new candidate-collection loop (lines 42–59) duplicates the snapshot filtering conditions (empty ID, AMI-used, retention cutoff) from the main target loop below it (lines 67–87). Verify these stay in sync — if they diverge, a volume ID could be silently skipped or incorrectly included. Consider whether a single-pass approach would be safer.
  • Batch fallback worst-case: When most/all volumes are deleted (common for orphaned snapshots), the batch call always fails and falls back to individual calls, making this potentially slower than the original N+1 pattern (batch call + N individual calls). Verify this tradeoff is acceptable for the expected workload, or consider filtering known-missing IDs differently.
  • Batch size of 200: Confirm that 200 is within the AWS DescribeVolumes API limit for VolumeIds. The AWS docs should be checked as the source of truth.
  • Test coverage: Existing tests pass but only exercise 1–2 snapshots. There are no dedicated tests for the batch path, the fallback path, or multiple snapshots sharing the same volume ID. Consider adding coverage.

Notes


Open with Devin

Replace N+1 DescribeVolumes API calls (one per orphaned snapshot) with a
single batch lookup that checks all referenced volume IDs at once.

When a batch call fails due to a missing volume in the set, the function
falls back to individual lookups only for that batch, preserving
correctness while still being far more efficient in the common case.

Co-Authored-By: unknown <>
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@dannysteenman dannysteenman merged commit d1499c9 into main Feb 23, 2026
3 checks passed
@dannysteenman dannysteenman deleted the devin/1771790520-batch-volume-exists branch February 23, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant