You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/09/19 20:37:09 UTC

[GitHub] [airflow] potiuk opened a new pull request #18362: Fix random deadlocks in MSSQL database

potiuk opened a new pull request #18362:
URL: https://github.com/apache/airflow/pull/18362


   Default isolation level for the MSSQL database is READ_COMMITTED
   but its implementation in MSSQL might cause random deadlocks for
   select queries - because by default MSSQL uses shared locks
   for READ_COMMITTED transaction isolation that might cause
   deadlocks with the UPDATE statements which create exclusive
   locks.
   
   This change switches the MSSQL DB used during tests to have
   READ_COMMITTED_SNAPSHOTS enabled and it adds check at the Airflow
   startup to make sure that if the MSSQL database is used, the
   READ_COMMITTED_SNAPSHOTS is set and documentation is added to
   describe it.
   
   It also allows to revert removal of 2017 version of mssql which
   was done in the faith that removal of 2017 will remove the
   flaky random test failures.
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711802455



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -20,12 +20,12 @@ services:
   airflow:
     environment:
       - BACKEND=mssql
-      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
-      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
       - AIRFLOW__CORE__EXECUTOR=LocalExecutor
     depends_on:
-      mssql:
-        condition: service_healthy
+      - mssqlsetup

Review comment:
       By using this condition, the container will not be started until another container is stopped, so you can do additional initiations steps before running the main container, e.g. initialize the database, get dependencies and then start the main container.
   Here are docs about it:
   https://github.com/compose-spec/compose-spec/blob/master/spec.md#long-syntax-1
   This is supported starting with the 1.29.0 (2021-04-06) version. https://docs.docker.com/compose/release-notes/#1290
   Here is another real-life example with node app:
   https://github.com/KlubJagiellonski/pola-web/blob/8c93e0b79231c2b92a984099df3209e287ac9458/docker-compose.yaml#L17-L18
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#issuecomment-922533669


   cc: @aneesh-joseph 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711800824



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -20,12 +20,12 @@ services:
   airflow:
     environment:
       - BACKEND=mssql
-      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
-      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
       - AIRFLOW__CORE__EXECUTOR=LocalExecutor
     depends_on:
-      mssql:
-        condition: service_healthy
+      - mssqlsetup

Review comment:
       What do you think about it? 
   
   when I try to use it in IntelliJ it tells me it's not expected here. Can you tell me whether it is a good idea to use it? Is it supported / usable in all compose versions? 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk merged pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #18362:
URL: https://github.com/apache/airflow/pull/18362


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711817305



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -38,3 +40,14 @@ services:
       timeout: 10s
       retries: 10
     restart: always
+  mssqlsetup:
+    image: mcr.microsoft.com/mssql/server:${MSSQL_VERSION}
+    depends_on:
+      mssql:
+        condition: service_healthy
+    entrypoint:
+      - bash
+      - -c
+      - opt/mssql-tools/bin/sqlcmd -S mssql -U sa -P Airflow123 -i /mssql_create_airflow_db.sql | true

Review comment:
       Yep. I noticed and fixed it at about the same time you did




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711810390



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -38,3 +40,14 @@ services:
       timeout: 10s
       retries: 10
     restart: always
+  mssqlsetup:
+    image: mcr.microsoft.com/mssql/server:${MSSQL_VERSION}
+    depends_on:
+      mssql:
+        condition: service_healthy
+    entrypoint:
+      - bash
+      - -c
+      - opt/mssql-tools/bin/sqlcmd -S mssql -U sa -P Airflow123 -i /mssql_create_airflow_db.sql | true

Review comment:
       Should we have double `|` here ie. `|` => `||`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711804728



##########
File path: docs/apache-airflow/howto/set-up-database.rst
##########
@@ -226,6 +227,27 @@ For more information regarding setup of the PostgresSQL connection, see `Postgre
 
      hba
 
+Setting up a MsSQL Database
+---------------------------
+
+You need to create a database and a database user that Airflow will use to access this database.
+In the example below, a database ``airflow_db`` and user  with username ``airflow_user`` with password ``airflow_pass`` will be created.

Review comment:
       ```suggestion
   In the example below, a database ``airflow_db`` and user  with username ``airflow_user`` with password ``airflow_pass123%`` will be created.
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711802455



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -20,12 +20,12 @@ services:
   airflow:
     environment:
       - BACKEND=mssql
-      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
-      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
       - AIRFLOW__CORE__EXECUTOR=LocalExecutor
     depends_on:
-      mssql:
-        condition: service_healthy
+      - mssqlsetup

Review comment:
       By using this condition, the container will not be started until another container is stopped, so you can do additional initiations steps before running the main container, e.g. initialize the database, get dependencies, and then start the main container.
   Here are docs about it:
   https://github.com/compose-spec/compose-spec/blob/master/spec.md#long-syntax-1
   This is supported starting with the 1.29.0 (2021-04-06) version. https://docs.docker.com/compose/release-notes/#1290
   Here are another real-life example with node app:
   https://github.com/KlubJagiellonski/pola-web/blob/8c93e0b79231c2b92a984099df3209e287ac9458/docker-compose.yaml#L17-L18
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] SamWheating commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
SamWheating commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711842380



##########
File path: tests/core/test_sqlalchemy_config.py
##########
@@ -77,7 +77,7 @@ def test_sql_alchemy_connect_args(
         with conf_vars(config):
             settings.configure_orm()
             engine_args = {}
-            if settings.SQL_ALCHEMY_CONN.startswith('mysql'):
+            if settings.SQL_ALCHEMY_CONN.startswith('mysql') or settings.SQL_ALCHEMY_CONN.startswith('mssql'):

Review comment:
       ```suggestion
               if settings.SQL_ALCHEMY_CONN.startswith(('mysql', 'mssql')):
   ```
   
   Since `startswith()` can accept a tuple of strings




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711802946



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -20,12 +20,12 @@ services:
   airflow:
     environment:
       - BACKEND=mssql
-      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
-      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
       - AIRFLOW__CORE__EXECUTOR=LocalExecutor
     depends_on:
-      mssql:
-        condition: service_healthy
+      - mssqlsetup

Review comment:
       Ah - I see, I think I figured it out :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711802537



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -20,12 +20,12 @@ services:
   airflow:
     environment:
       - BACKEND=mssql
-      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
-      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
       - AIRFLOW__CORE__EXECUTOR=LocalExecutor
     depends_on:
-      mssql:
-        condition: service_healthy
+      - mssqlsetup

Review comment:
       I do not know basically how to make dependency on two services - one with a condition and one without  - it does not seem to be supported to depend on one service that has no condition and another service that depends on "just service" - the first one requires dict, the second list, but I could not find a way how to make both work.
   
   that 's why I made `airflow` depends on both mssql and mssqlsetup (unconditionally) but also make mssqlsetup depends conditionally on healthy status of mssql 
   
   I think it does the job nicely - but if you can find a better solution - I am all ears.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711800393



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -20,12 +20,12 @@ services:
   airflow:
     environment:
       - BACKEND=mssql
-      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
-      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
       - AIRFLOW__CORE__EXECUTOR=LocalExecutor
     depends_on:
-      mssql:
-        condition: service_healthy
+      - mssqlsetup

Review comment:
       What do you think about add condition - `service_completed_successfully`?
   https://github.com/apache/airflow/blob/14b3e66ad5e2325eb2ee7b9c6661ff7b636d017d/docs/apache-airflow/start/docker-compose.yaml#L114-L115




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711801973



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -20,12 +20,12 @@ services:
   airflow:
     environment:
       - BACKEND=mssql
-      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
-      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/master?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CORE__SQL_ALCHEMY_CONN=mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
+      - AIRFLOW__CELERY__RESULT_BACKEND=db+mssql+pyodbc://sa:Airflow123@mssql:1433/airflow?driver=ODBC+Driver+17+for+SQL+Server
       - AIRFLOW__CORE__EXECUTOR=LocalExecutor
     depends_on:
-      mssql:
-        condition: service_healthy
+      - mssqlsetup

Review comment:
       `service_healty` works - but when I tried to replace `service_healthy` with `service completed successfully` - it hangs indefinitely, but maybe this is something I am doing wrong - if you could check how to make use of it, it would be great 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711801218



##########
File path: docs/apache-airflow/howto/set-up-database.rst
##########
@@ -226,6 +226,27 @@ For more information regarding setup of the PostgresSQL connection, see `Postgre
 
      hba
 
+Setting up a MsSQL Database

Review comment:
       Yep. Good point.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711810390



##########
File path: scripts/ci/docker-compose/backend-mssql.yml
##########
@@ -38,3 +40,14 @@ services:
       timeout: 10s
       retries: 10
     restart: always
+  mssqlsetup:
+    image: mcr.microsoft.com/mssql/server:${MSSQL_VERSION}
+    depends_on:
+      mssql:
+        condition: service_healthy
+    entrypoint:
+      - bash
+      - -c
+      - opt/mssql-tools/bin/sqlcmd -S mssql -U sa -P Airflow123 -i /mssql_create_airflow_db.sql | true

Review comment:
       Should we have double `|` here?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711957503



##########
File path: tests/core/test_sqlalchemy_config.py
##########
@@ -77,7 +77,7 @@ def test_sql_alchemy_connect_args(
         with conf_vars(config):
             settings.configure_orm()
             engine_args = {}
-            if settings.SQL_ALCHEMY_CONN.startswith('mysql'):
+            if settings.SQL_ALCHEMY_CONN.startswith('mysql') or settings.SQL_ALCHEMY_CONN.startswith('mssql'):

Review comment:
       TIL! Thanks. i had no idea, it's just one sentence in the docs mentioning it :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711800510



##########
File path: docs/apache-airflow/howto/set-up-database.rst
##########
@@ -226,6 +226,27 @@ For more information regarding setup of the PostgresSQL connection, see `Postgre
 
      hba
 
+Setting up a MsSQL Database
+---------------------------
+
+You need to create a database and a database user that Airflow will use to access this database.

Review comment:
       I think it's not - once we release 2.2 it's not going to be experimental




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711800094



##########
File path: docs/apache-airflow/howto/set-up-database.rst
##########
@@ -226,6 +226,27 @@ For more information regarding setup of the PostgresSQL connection, see `Postgre
 
      hba
 
+Setting up a MsSQL Database

Review comment:
       Can you also mention in section above - "Choosing database backend"? 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#discussion_r711800173



##########
File path: docs/apache-airflow/howto/set-up-database.rst
##########
@@ -226,6 +226,27 @@ For more information regarding setup of the PostgresSQL connection, see `Postgre
 
      hba
 
+Setting up a MsSQL Database
+---------------------------
+
+You need to create a database and a database user that Airflow will use to access this database.

Review comment:
       Should we add warning that support for MSSQL is experimental?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #18362: Fix random deadlocks in MSSQL database

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18362:
URL: https://github.com/apache/airflow/pull/18362#issuecomment-922722400


   @aneesh-joseph  -> if you have setup MSSQL before, setting the "READ_COMMITTED_SNAPSHOT" database option is CRUCIAL to avoid deadlocks on read. It is apparently set on by default on Azure databases, but it is disabled by default on regular mssql databases. in 2.2.0 we are checking if the option is enabled for the airflow database and fail if not.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org