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

Fix race condition in draft phase #131

Merged

Conversation

sp-tkerlavage
Copy link
Contributor

I have a somewhat large project that I'm trying to refactor. When I was running dbt-osmosis, I was getting this error randomly:

ERROR    Failed to draft project structure update plan for model.my_project.some_model: 'models'         osmosis.py:610

The error message isn't very helpful. This same error would be reported for many different models, while other models would succeed during this step.

Running dbt-osmosis for a single model at a time produced no such error -- even on models that were erroring when dbt-osmosis was invoked across many models.

Looking at the traceback a better picture

ERROR    Failed to draft project structure update plan for model.my_project.some_model: 'models'         osmosis.py:610
         ╭───────────────────────────────────────────── Traceback (most recent call last) ─────────────────────────────────────────────╮               
         │ /Users/me/Projects/dbt-osmosis/src/dbt_osmosis/core/osmosis.py:559 in _draft                                   │               
         │                                                                                                                             │               
         │    556 │   │   │   │   │   # Augment Documented Model                                                                       │               
         │    557 │   │   │   │   │   augmented_model = self.augment_existing_model(documented_model,                                  │               
         │        node)                                                                                                                │               
         │    558 │   │   │   │   │   with self.mutex:                                                                                 │               
         │ ❱  559 │   │   │   │   │   │                                                                                                │               
         │        blueprint[schema_file.target].output["models"].append(augmented_model)                                               │               
         │    560 │   │   │   │   │   │   # Target to supersede current                                                                │               
         │    561 │   │   │   │   │   │   blueprint[schema_file.target].supersede.setdefault(                                          │               
         │    562 │   │   │   │   │   │   │   schema_file.current, []                                                                  │               
         ╰─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯               
         KeyError: 'models'

I think this error is the result of the cleanup that happens in the _draft method. Specifically this code

            for k in blueprint:
                # Remove if sources or models are empty
                if blueprint[k].output.get("sources", None) == []:
                    del blueprint[k].output["sources"]
                if blueprint[k].output.get("models", None) == []:
                    del blueprint[k].output["models"]

This seems like its some sort of race condition where if the cleanup happens first, and another thread tries to append, it cant because the key doesnt exist.

Moving this cleanup out of the _draft method into its own method and calling it after draft_project_structure_update_plan seems to fix the issue.

@z3z1ma z3z1ma merged commit 984121a into z3z1ma:main Mar 11, 2024
1 of 2 checks passed
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.

2 participants