[GitHub] rhunwicks commented on issue #1260: Migrations for 0.11 hangs with posgresql

[gi...@git.apache.org]

rhunwicks commented on issue #1260: Migrations for 0.11 hangs with posgresql 
URL: https://github.com/apache/incubator-superset/issues/1260#issuecomment-334115725
 
 
   tldr; adding `sm.get_session.close()` to the bottom of `superset/__init__.py` fixes it.
   
   I have also experienced this bug. I think it happens because:
   
   * `superset/__init__.py` calls
   * `ConnectorRegistry.register_sources(module_datasource_map)` and that in turn does
   * `__import__(module_name, fromlist=class_names)` and that ends up calling
   * `flask_appbuilder.base.add_view()` which eventually calls
   * `sm.add_permissions_menu()` which calls
   * `find_permission_view_menu()` which does
   * `return self.get_session.query(self.permissionview_model).filter_by(permission=permission, view_menu=view_menu).first()`
   
   The final call is executing a query against the database, and because of the Python dbapi transaction semantics we now have an open, idle transaction which blocks the `CREATE TABLE`. That happens because the table has a foreign key to `ab_user` and so creating the constraint on the database requires `AccessExclusiveLock` on `ab_user`, and the transaction can't get that if there is another open transaction somewhere that has selected it somehow.
   
   It's not clear what the best place to fix it is.
   * you could file a bug against Flask-Appbuilder and propose that `add_view` should always leave the database without an open transaction - i.e. you call `self.sm.get_session.close()` at the bottom of `add_view` before `return baseview`
   * you could try and fix it in `appbuilder.security.manager` by closing the session before returning the found permission/view - but that will be invasive and you would need to apply it to all the `find_` methods and it might break other things if we are calling `find` midway through a transaction to create a new menu or whatever
   * you could fix it in `superset/__init__.py` by calling `sm.get_session.close` at the bottom of the file - that seems safe as we can be sure we shouldn't have any transactions in flight at that point
   * you might also be able to make it work by getting the migration command to use the same session as the security manager
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services