Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Sep 19, 2021

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.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@potiuk
Copy link
Member Author

potiuk commented Sep 19, 2021

cc: @aneesh-joseph

@potiuk potiuk added the full tests needed We need to run full set of tests for this PR to merge label Sep 19, 2021
@potiuk potiuk force-pushed the fix-deadlocks-on-mssql branch from 29cdaaf to e20fc4b Compare September 19, 2021 20:38
@potiuk potiuk force-pushed the fix-deadlocks-on-mssql branch 4 times, most recently from d72c78a to 00ef7fe Compare September 19, 2021 21:06
@potiuk potiuk linked an issue Sep 19, 2021 that may be closed by this pull request
@potiuk potiuk requested a review from eladkal September 19, 2021 21:33
@potiuk potiuk force-pushed the fix-deadlocks-on-mssql branch from edf3c5a to 1eed784 Compare September 19, 2021 22:05
@potiuk potiuk force-pushed the fix-deadlocks-on-mssql branch 2 times, most recently from b788061 to 3b2656f Compare September 19, 2021 22:40
@potiuk potiuk force-pushed the fix-deadlocks-on-mssql branch 2 times, most recently from df806d3 to f3d2da7 Compare September 20, 2021 08:19
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.

Also Fixes: apache#17018
@potiuk potiuk force-pushed the fix-deadlocks-on-mssql branch from f3d2da7 to bd2d435 Compare September 20, 2021 08:20
@potiuk
Copy link
Member Author

potiuk commented Sep 20, 2021

@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.

@potiuk potiuk merged commit f76eaec into apache:main Sep 20, 2021
@potiuk potiuk deleted the fix-deadlocks-on-mssql branch September 20, 2021 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools full tests needed We need to run full set of tests for this PR to merge kind:documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing documentation about MSSQL setting up

3 participants