Optimize Athena partition deletions for insert_overwrite strategy - #1558
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change in the dbt-athena package. For details on how to document a change, see the Contributing Guide. |
There was a problem hiding this comment.
Pull request overview
This PR optimizes insert_overwrite incremental runs in the dbt-athena adapter by reducing the number of AWS API calls required to delete overlapping partitions (batching both Glue partition deletions and S3 object deletions), addressing performance bottlenecks described in issue #1125.
Changes:
- Update the incremental helper macro to call
adapter.clean_up_partitions()once with a list of partition predicates (instead of one call per partition). - Enhance
AthenaAdapter.clean_up_partitions()to chunk GlueGetPartitionsexpressions and to batch S3 + Glue deletions. - Add unit + functional tests to validate batching/chunking behavior, plus a new
chunk_iterable()utility.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
dbt-athena/src/dbt/adapters/athena/impl.py |
Implements chunked partition fetching, batch Glue deletions, and bulk S3 deletions. |
dbt-athena/src/dbt/adapters/athena/utils.py |
Adds chunk_iterable() to chunk generator/iterable inputs. |
dbt-athena/src/dbt/include/athena/macros/materializations/models/incremental/helpers.sql |
Switches partition cleanup from per-partition calls to a single batched call. |
dbt-athena/tests/unit/test_adapter.py |
Adds/updates unit tests for list-input compatibility, large-partition chunking, and S3 delete error handling. |
dbt-athena/tests/functional/adapter/test_partition_chunking.py |
Adds an integration test to exercise Glue expression-length chunking in a real run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…errors The Glue batch_delete_partition API can return per-partition failures in the response body without raising an exception. Capture the response, log each failed partition with its error details, and raise DbtRuntimeError so failures are not silently ignored.
…gging Group S3 paths by bucket before bulk deletion so that partitions spread across multiple buckets are handled correctly. Previously an error was raised when paths spanned buckets, which was a behavioral regression from the prior per-partition approach. Also replace full delete_objects response logging with a concise summary (deleted count and error count) to avoid flooding logs when deleting up to 1000 objects per batch.
… expression limit The expression chunking logic splits conditions across multiple API calls when the combined expression would exceed 2048 characters, but it had no guard for a single condition that already exceeds the limit on its own. Add an explicit check so users get a clear DbtRuntimeError instead of a cryptic Glue API failure.
|
Addressed the copilot review comments (with relevant test cases added). |
There was a problem hiding this comment.
Pull request overview
This PR optimizes insert_overwrite incremental partition cleanup for the dbt-athena adapter by batching S3 object deletions and Glue partition deletions to dramatically reduce API calls and runtime (resolves #1125).
Changes:
- Update
clean_up_partitionsto acceptUnion[str, List[str]]and chunk GlueGetPartitionsexpressions to respect the 2048-character limit. - Add batched S3 deletion via
bulk_delete_from_s3()(up to 1000 objects per call) and batched Glue partition deletion (25 per call). - Add unit + functional tests covering batching, chunking, multi-bucket deletes, and error surfacing.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| dbt-athena/src/dbt/adapters/athena/impl.py | Implements chunked partition retrieval plus batched S3/Glue deletions; adds bulk S3 delete API. |
| dbt-athena/src/dbt/adapters/athena/utils.py | Adds chunk_iterable() to chunk generators/iterables for streaming processing. |
| dbt-athena/src/dbt/include/athena/macros/materializations/models/incremental/helpers.sql | Switches macro to call clean_up_partitions once with a list of partition predicates. |
| dbt-athena/tests/unit/test_adapter.py | Updates/extends unit coverage for list input, chunking at scale, S3/Glue error handling, and multi-bucket deletes. |
| dbt-athena/tests/functional/adapter/test_partition_chunking.py | Adds functional coverage to validate expression chunking with real AWS API constraints. |
| dbt-athena/.changes/unreleased/Under the Hood-20260223-225659.yaml | Adds changelog entry for the optimization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
colin-k-rogers
left a comment
There was a problem hiding this comment.
On testing options: I don't think we need a specific integration test for chunking, ideally this is validated as part of a test of insert_overwrite
One question on the order of operations here but this looks just about good to ship
Did you mean modifying some of the existing integration tests (tests/functional/adapter/something) or just making sure that unit tests have the desired coverage? If you would still like to cover this in an integration test (some existing one, right?), then would you like to optimize the speed by mocking (as proposed in PR desc)? I suppose an extra 2 minutes is too much for this :) |
|
I checked for different options for testing. Maybe this could help with the decision: What the new functional test covers that unit tests don't: Unit tests mock AWS responses and verify chunking logic in isolation. The functional test is the only place that verifies chunking integrates correctly Existing insert_overwrite functional tests: Folding chunking into the existing test: |
@juhoautio-rovio if we can do this that would be great |
…verwrite test Removes the standalone test_partition_chunking.py (150 partitions, 2+ min) and folds chunking coverage into TestUniqueTmpTableSuffix by monkeypatching GET_PARTITIONS_API_EXPRESSION_MAX_LENGTH=60, forcing 2 Glue API calls with just 4 partitions.
…t-labs#1558) Co-authored-by: Colin Rogers <111200756+colin-rogers-dbt@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
resolves #1125
docs: "N/A"
Problem
The dbt-athena adapter currently deletes metadata and files of overlapping partitions by making individual API calls for each partition.
This naive approach results in extraneous API calls for incremental models that process more than a few partitions. For example, a model with 150 partitions requires 300+ API calls (150+ to Glue, 150+ to S3), causing partition deletion to dominate the total execution time. In real-world testing,
clean_up_partitionsconsumed 13.5 minutes out of a 15-minute total runtime, making partition deletion the primary performance bottleneck.Solution
Significantly improves performance of
insert_overwriteincremental materializations by optimizing S3 and Glue API usage when deleting partitions.Key Changes
bulk_delete_from_s3()method batches up to 1000 objects per API callbatch_delete_partitionAPI (25 partitions per call)Performance Impact
API call reduction - for example, if there are 150 partitions to handle:
Real-world execution time (150-partition incremental model):
Testing
Manual validation
Tested in a dbt project by patching dbt-athena adapter in the project venv and running:
dbt run -s my_daily_incremental_model --vars '{"start_date": "2024-01-10", "end_date": "2025-05-11"}'In other words, the
clean_up_partitionspart was ~10x faster.Question about a slow integration test
The new
test_partition_chunking.pytakes approximately 2+ minutes to run. Is that too much?Reason for the long duration is creating and processing 150 partitions with real AWS API calls. The Glue GetPartitions API has a 2048 character limit for the Expression parameter. This test creates enough partitions to trigger code path that splits to have more than a single chunk. There's no way to trigger that scenario without creating many partitions or modifying the runtime code.
Some options to consider:
GET_PARTITIONS_API_EXPRESSION_MAX_LENGTH(default 2048) to use a smaller value during the test, so that a minimal number of partitions are needed to trigger chunking.GET_PARTITIONS_API_EXPRESSION_MAX_LENGTHconfigurable and set a smaller value during the test. This would not be needed in actual usage though, so it would be adding noise to the set of available configurations.Future Improvements
Consider parallelizing S3 API calls for additional performance gains.
Checklist
clean_up_partitionssignature change fromstrtoUnion[str, List[str]]was specified by @colin-rogers-dbt in [Feature] Optimize Athena insert overwrite deletions by using batch operations #1125 (comment)bulk_delete_from_s3()method is marked@availablefor potential future use; the annotation can be removed if not desired for the public API