Delay instantiation of User and Spawner objects

Avoids instantiating too many objects before they are used

- deletes Spawner instances after they stop to avoid lingering instances
- use user_dict cache more often instead of db queries
- check for empty spawners dict to avoid a few Spawner instantiations

jupyterhub/apihandlers/base.py

@@ -104,7 +104,10 @@ class APIHandler(BaseHandler):
             'pending': None,
             'last_activity': user.last_activity.isoformat(),
         }
+        if '' in user.spawners:
             model['pending'] = user.spawners[''].pending or None
+        else:
+            model['pending'] = False
 
         if self.allow_named_servers:
             servers = model['servers'] = {}

jupyterhub/apihandlers/users.py

@@ -81,7 +81,7 @@ class UserListAPIHandler(APIHandler):
                 yield gen.maybe_future(self.authenticator.add_user(user))
             except Exception as e:
                 self.log.error("Failed to create user: %s" % name, exc_info=True)
-                del self.users[user]
+                self.users.delete(user)
                 raise web.HTTPError(400, "Failed to create user %s: %s" % (name, str(e)))
             else:
                 created.append(user)
@@ -132,7 +132,7 @@ class UserAPIHandler(APIHandler):
         except Exception:
             self.log.error("Failed to create user: %s" % name, exc_info=True)
             # remove from registry
-            del self.users[user]
+            self.users.delete(user)
             raise web.HTTPError(400, "Failed to create user: %s" % name)
 
         self.write(json.dumps(self.user_model(user)))
@@ -155,7 +155,7 @@ class UserAPIHandler(APIHandler):
 
         yield gen.maybe_future(self.authenticator.delete_user(user))
         # remove from registry
-        del self.users[user]
+        self.users.delete(user)
 
         self.set_status(204)
 

jupyterhub/app.py

@@ -1034,6 +1034,8 @@ class JupyterHub(Application):
                     without notifying JupyterHub.
                     """))
         db.commit()
+        # expunge User objects from the db session
+        db.expunge_all()
 
         # The whitelist set and the users in the db are now the same.
         # From this point on, any user changes should be done simultaneously
@@ -1060,6 +1062,8 @@ class JupyterHub(Application):
                     db.add(user)
                 group.users.append(user)
         db.commit()
+        # expunge Group objects from the db session
+        db.expunge_all()
 
     @gen.coroutine
     def _add_tokens(self, token_dict, kind):
@@ -1263,7 +1267,7 @@ class JupyterHub(Application):
                 continue
             seen.add(orm_spawner.user_id)
             orm_user = orm_spawner.user
-            self.users[orm_user.id] = user = User(orm_user, self.tornado_settings)
+            user = self.users.add(orm_user)
             self.log.debug("Loading state for %s from db", user.name)
             for name, spawner in user.spawners.items():
                 f = check_spawner(user, name, spawner)
@@ -1271,6 +1275,15 @@ class JupyterHub(Application):
 
         # await checks after submitting them all
         yield gen.multi(check_futures)
+        to_expunge = []
+        for user in self.users.values():
+            if not user.active:
+                to_expunge.append(user)
+        if to_expunge:
+            self.log.debug("expunging users from db session: %s",
+                           ', '.join(u.name for u in to_expunge))
+            for user in to_expunge:
+                del self.users[user.id]
 
         db.commit()
         # only perform this query if we are going to log it

jupyterhub/handlers/base.py

@@ -258,7 +258,6 @@ class BaseHandler(RequestHandler):
             self.db.add(u)
             self.db.commit()
             user = self._user_from_orm(u)
-            self.authenticator.add_user(user)
         return user
 
     def clear_login_cookie(self, name=None):

jupyterhub/proxy.py

@@ -262,13 +262,11 @@ class Proxy(LoggingConfigurable):
         """
         db = self.db
         futures = []
-        for orm_service in db.query(Service):
+        for service in service_dict.values():
-            service = service_dict[orm_service.name]
             if service.server:
                 futures.append(self.add_service(service))
         # wait after submitting them all
-        for f in futures:
+        yield gen.multi(futures)
-            yield f
 
     @gen.coroutine
     def add_all_users(self, user_dict):
@@ -278,14 +276,12 @@ class Proxy(LoggingConfigurable):
         """
         db = self.db
         futures = []
-        for orm_user in db.query(User):
+        for user in user_dict.values():
-            user = user_dict[orm_user]
             for name, spawner in user.spawners.items():
                 if spawner.ready:
                     futures.append(self.add_user(user, name))
         # wait after submitting them all
-        for f in futures:
+        yield gen.multi(futures)
-            yield f
 
     @gen.coroutine
     def check_routes(self, user_dict, service_dict, routes=None):
@@ -309,8 +305,7 @@ class Proxy(LoggingConfigurable):
                 self.log.warning("Updating default route %s → %s", route['target'], hub.host)
                 futures.append(self.add_hub_route(hub))
 
-        for orm_user in db.query(User):
+        for user in user_dict.values():
-            user = user_dict[orm_user]
             for name, spawner in user.spawners.items():
                 if spawner.ready:
                     spec = spawner.proxy_spec
@@ -333,14 +328,8 @@ class Proxy(LoggingConfigurable):
         # check service routes
         service_routes = {r['data']['service']: r
                           for r in routes.values() if 'service' in r['data']}
-        for orm_service in db.query(Service).filter(Service.server != None):
+        for service in service_dict.values():
-            service = service_dict[orm_service.name]
             if service.server is None:
-                # This should never be True, but seems to be on rare occasion.
-                # catch filter bug, either in sqlalchemy or my understanding of
-                # its behavior
-                self.log.error(
-                    "Service %s has no server, but wasn't filtered out.", service)
                 continue
             good_routes.add(service.proxy_spec)
             if service.name not in service_routes:

jupyterhub/user.py

@@ -34,6 +34,15 @@ class UserDict(dict):
     def db(self):
         return self.db_factory()
 
+    def from_orm(self, orm_user):
+        return User(orm_user, self.settings)
+
+    def add(self, orm_user):
+        """Add a user to the UserDict"""
+        if orm_user.id not in self:
+            self[orm_user.id] = self.from_orm(orm_user)
+        return self[orm_user.id]
+
     def __contains__(self, key):
         if isinstance(key, (User, orm.User)):
             key = key.id
@@ -63,18 +72,30 @@ class UserDict(dict):
                 orm_user = self.db.query(orm.User).filter(orm.User.id == id).first()
                 if orm_user is None:
                     raise KeyError("No such user: %s" % id)
-                user = self[id] = User(orm_user, self.settings)
+                user = self.add(orm_user)
-            return dict.__getitem__(self, id)
+            else:
+                user = dict.__getitem__(self, id)
+            return user
         else:
             raise KeyError(repr(key))
 
     def __delitem__(self, key):
+        user = self[key]
+        for orm_spawner in user.orm_user._orm_spawners:
+            if orm_spawner in self.db:
+                self.db.expunge(orm_spawner)
+        if user.orm_user in self.db:
+            self.db.expunge(user.orm_user)
+        dict.__delitem__(self, user.id)
+
+    def delete(self, key):
+        """Delete a user from the cache and the database"""
         user = self[key]
         user_id = user.id
-        db = self.db
+        self.db.delete(user)
-        db.delete(user.orm_user)
+        self.db.commit()
-        db.commit()
+        # delete from dict after commit
-        dict.__delitem__(self, user_id)
+        del self[user_id]
 
     def count_active_users(self):
         """Count the number of user servers that are active/pending/ready
@@ -237,11 +258,15 @@ class User(HasTraits):
     @property
     def running(self):
         """property for whether the user's default server is running"""
+        if not self.spawners:
+            return False
         return self.spawner.ready
 
     @property
     def active(self):
         """True if any server is active"""
+        if not self.spawners:
+            return False
         return any(s.active for s in self.spawners.values())
 
     @property
@@ -371,7 +396,7 @@ class User(HasTraits):
         # wait for spawner.start to return
         try:
             # run optional preparation work to bootstrap the notebook
-            yield gen.maybe_future(self.spawner.run_pre_spawn_hook())
+            yield gen.maybe_future(spawner.run_pre_spawn_hook())
             f = spawner.start()
             # commit any changes in spawner.start (always commit db changes before yield)
             db.commit()
@@ -524,3 +549,5 @@ class User(HasTraits):
             except Exception:
                 self.log.exception("Error in Authenticator.post_spawn_stop for %s", self)
             spawner._stop_pending = False
+            # pop the Spawner object
+            self.spawners.pop(server_name)