Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: skip_workgroup_check setting to reduce AWS throttling #713

Merged
merged 7 commits into from
Sep 12, 2024

Conversation

amacal
Copy link
Contributor

@amacal amacal commented Sep 5, 2024

Description

The adapter performs a GetWorkGroup operation on each thread, which is later cached. When dbt build is started for 10 independent models, it issues 10 AWS requests to get the same information. If dbt is orchestrated via Airflow to run 32 dbt tasks concurrently, it sends 320 GetWorkGroup requests, causing throttling on the AWS side.

This PR introduces a skip_workgroup_check setting, which instructs dbt to skip checking if a WorkGroup contains an enforced output location.

Checklist

  • You followed contributing section
  • You kept your Pull Request small and focused on a single feature or bug fix.
  • You added unit testing when necessary
  • You added functional testing when necessary

@nicor88
Copy link
Contributor

nicor88 commented Sep 5, 2024

@amacal implementation looks good.

Please have a look at the ci, precommit checks are failing.
Provide unit testing for is_work_group_output_location_enforced function cover the introduced feature.

Nice to have, and not mandatory, add some "integration tests" to fully test the feature in a real environment.

dbt/adapters/athena/impl.py Outdated Show resolved Hide resolved
dbt/adapters/athena/impl.py Outdated Show resolved Hide resolved
@nicor88
Copy link
Contributor

nicor88 commented Sep 6, 2024

@amacal plese remember what I suggested before:

  • add unit tests
  • add integration tests
  • document the new parameter in the docs

Without all these 3 conditions, we cannot merge. Thanks

@amacal
Copy link
Contributor Author

amacal commented Sep 6, 2024

I added a unit test and documented new parameter in the documentation. I cannot do functional test, because I cannot push it via org AWS account.

@nicor88
Copy link
Contributor

nicor88 commented Sep 6, 2024

@svdimchenko @Jrmyy could you have a look when you have some time? I'm currently away from keyboard.
Thanks

@nicor88 nicor88 added the enable-functional-tests Label to trigger functional testing label Sep 6, 2024
@nicor88
Copy link
Contributor

nicor88 commented Sep 6, 2024

@amacal implementation looks fine, have a look at the CI, there is a failure

@amacal amacal changed the title feat: work_group_enforced setting to reduce AWS throttling feat: skip_workgroup_check setting to reduce AWS throttling Sep 6, 2024
@amacal
Copy link
Contributor Author

amacal commented Sep 10, 2024

Is there anything I need to do to have it merged?

@nicor88
Copy link
Contributor

nicor88 commented Sep 11, 2024

@amacal you changes looks good. As you can see repository ownership is being moved to dbtlabs, they are the official owners.
@colin-rogers-dbt could you have a look here?

@colin-rogers-dbt colin-rogers-dbt merged commit d2bc485 into dbt-labs:main Sep 12, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enable-functional-tests Label to trigger functional testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants