Updating liquid clustering on incremental runs breaks concurrency for incremental tables

mmansikka commented 1 month ago

Describe the bug

Since the additions of

updating the liquid clustering https://github.com/databricks/dbt-databricks/pull/765 and
updating column descriptions https://github.com/databricks/dbt-databricks/commit/198b56900a18c957388984b20687d5a32d01c530

We no break concurrency because of alter table statements running at the same time as another process is writing to the same table

Steps To Reproduce

Run several dbt processes at the same time for example when using different source systems dbt run -s common_table --vars '{"source_systems": ["SOURCE_1"]}' dbt run -s common_table --vars '{"source_systems": ["SOURCE_2"]}' dbt run -s common_table --vars '{"source_systems": ["SOURCE_3]}'

Expected behavior

By default do not run liquid clustering updates or column updates when incremental is run. This behavior should be controlled perhaps with a config parameter and if it is empty (default) then do not update column descriptions or liquid clustering columns. As a quick and dirty fix we added if not is_incremental() checks to incremental materialization here https://github.com/databricks/dbt-databricks/blob/52e9c7a379ccd3a0496c9e60e1493706596b1bf1/dbt/include/databricks/macros/materializations/incremental/incremental.sql#L113

    {% if tblproperties is not none and not is_incremental() %} {# override: add incremental check, to not break concurrency #}
        {% do apply_tblproperties(target_relation, tblproperties.tblproperties) %}
      {%- endif -%}
    {%- endif -%}
    {% if not is_incremental() %} {# override: add incremental check, to not break concurrency #}
        {% do persist_docs(target_relation, model, for_relation=True) %}
    {%- endif -%}

Screenshots and log output

System information

The output of dbt --version:

Core:
  - installed: 1.8.6
  - latest:    1.8.7 - Update available!

  Your version of dbt-core is out of date!
  You can find instructions for upgrading here:
  https://docs.getdbt.com/docs/installation

Plugins:
  - databricks: 1.8.6 - Update available!
  - spark:      1.8.0 - Up to date!

  At least one plugin is out of date or incompatible with dbt-core.
  You can find instructions for upgrading here:
  https://docs.getdbt.com/docs/installation

Additional context

Add any other context about the problem here.

benc-db commented 1 month ago

Is this concurrency within a single run, or are you talking about running multiple instances of dbt targetting the same table? The latter I don't think has ever been intentionally supported.

mmansikka commented 1 month ago

When running multiple instances of dbt targetting the same table. There has been quite a lot of work by databricks to support concurrent writes. It would be a shame if this is not supported by default, or there is no way to remove these concurrency breaking processes. I have discovered that the following break concurrency:

Optimize. If DATABRICKS_SKIP_OPTIMIZE is not set to true. See issue
updating liquid clustering (see above). Also this issue is related
updating docs generation (see above). Seems like documents are also updating (table comment) every run even though there are no changes.

If we would allow similar variables for docs generation and liquid clustering, we would be able to support optimized scheduled runs with multiple instances of dbt targetting the same table. See this issue

On config level this would even better than as vars because on larger project there is a need for granular settings and otherwise you would need to run multiple dbt runs. Also see discussion in the above issue.

databricks / dbt-databricks