dheerajturaga commented on PR #61155:
URL: https://github.com/apache/airflow/pull/61155#issuecomment-3868358388
> Cool! This looks good and reasonable. I would prefer the solution w/o
needed changes in core. Only thing we need to consider is that if DB manager is
not registered the modules might fail - in case of such we should have a good
error prompting telling the user that DB manager needs to be added to
configuration to create structures. As well as add this to documentation.
>
> Root cause of the discussion was that DB locks failed to be ackquired as
API server (each instance!) attempts to lock but needs to day a full DB lock.
How is this with Alembic? Does this check before locking if the DB is already
in target state? Or woul it be at the end the same problem with Alembic that an
exclusive lock (incl all problems) is needed?
If EdgeDBManager is not in external_db_managers config and someone runs
airflow db init, the edge tables won't be created. However EdgeExecutor.start()
will try to create them when executor starts anyway. Should we add a warning
instead of an error?
The only things you miss without the config registration are:
- `airflow db init / airflow db` upgrade won't manage edge tables
- `airflow db reset` won't drop edge tables
I think a warning there will inform the user of the recommended setup.
For the db lock, I have added fast-path check before acquiring the lock. If
proper db init/migrate with EdgeDBManager has occurred, multiple server
instances can start without contention.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
