[AIRFLOW-5417] Fix DB disconnects during webserver startup

[KevinYang21]

Make sure you have checked all steps below.

Jira

• My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
  • https://issues.apache.org/jira/browse/AIRFLOW-5417

Description

• Here are some details about my PR, including screenshots of any UI changes:
  The current workflow of creating an app in views.py is:

1. Create one DB session
2. Assign it to flask-appbuilder, use the session to help initializing the app.
3. Create DagBag
4. Reuse the same session to add permission using flask-appbuilder etc.
   • Reason for us reusing the session is because we use the scoped_session w/o a scope function, thus made the session a thread-local scope session, which will be reused until it is removed. Unfortunately flask-appbuilder doesn't remove the session after a query, e.g. 1.

Problem with this is that if during step 3, there's a DB disconnect, step 4 will fail because the session is still attached with the disconnected DB connection and not rolled back. It becomes a bigger problem if step 3 regularly takes a long time, more than the DB connection expire time, step 4 will always fail, which is the case for us currently.

Tried to move the import of views.py to the outer scope but that seem to fail multiple tests. The approach here is to remove the session from the registry and the next session call will just create a new one automatically.

Of course the proper way is to fix the session usage in flask-appbuilder but that would be a longer term fix.

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:
  Not really changing the behavior.

Commits

• My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation

• In case of new functionality, my PR adds documentation that describes how to use it.
  • All the public functions and the classes in the PR contain docstrings that explain what it does
  • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

[potiuk]

Very interesting one. I read a bit about scoped session in this context.

Just a thought:

Maybe a better solution (still temporary but a bit less invasive) will be to run init_views in a separate thread and join it later (if we need to join it at all). This way the session will not be removed from the thread_local scope and in case it is used later on it might be still used?

[KevinYang21]

@potiuk thanks for reviewing and the idea. Actually I think the cost is actually less if we just remove the session. We have to recreate the connection but either way we'll have to use two connections to finish all the init steps. Removing a session from a scoped session is very safe but I'm not that sure about the safeness of parallelly initializing the app.

[potiuk]

Fair :). It's LGTM for me but maybe someone with more insight of Flask Can also take a look.

[KevinYang21]

TY! I'll keep it open for two more days then.