#95 - Shard GridFS chunks if the Mongo instance supports it #142
Conversation
notebooker/serialization/mongo.py
Outdated
| conn.admin.command("enableSharding", self.database_name) | ||
| conn.admin.command({"shardCollection": f"{self.database_name}.notebook_data.chunks", "key": {"files_id": 1, "n": 1}}) | ||
| except pymongo.errors.OperationFailure: | ||
| pass |
There was a problem hiding this comment.
Worthy of a logger.error or logger.warning at least. I think we might instantiate this class quite often (i.e. almost every request that comes in) so maybe we should only attempt this once (at webapp startup in app.py perhaps)
notebooker/serialization/mongo.py
Outdated
| conn.admin.command("enableSharding", self.database_name) | ||
| conn.admin.command({"shardCollection": f"{self.database_name}.notebook_data.chunks", "key": {"files_id": 1, "n": 1}}) | ||
| except pymongo.errors.OperationFailure: | ||
| pass |
There was a problem hiding this comment.
I like the idea of a singleton, as long as we are sure it's threadsafe for incoming requests on the WSGI server. It is relatively stateless so should be fine, perhaps apart from the mongo connection?
re: the sharding - is this guaranteed to run for the first time in the application setup phase? Worst case is that the first request to the webserver does this (potentially long) setup unless we guarantee that we run this in app.py (e.g. by having a setup method which is only called there).
notebooker/web/app.py
Outdated
| flask_app = create_app(web_config) | ||
| flask_app = setup_app(flask_app, web_config) | ||
| serializer = get_serializer_from_cls(web_config.SERIALIZER_CLS, **web_config.SERIALIZER_CONFIG) | ||
| serializer.enable_sharding() |
There was a problem hiding this comment.
I think this is fine, but we need to make a note in the CHANGELOG that this will break any custom serializer which doesn't implement the enable_sharding() method. We don't provide a base class apart from the MongoResultSerializer so this is probably okay, but worth mentioning that this could cause issues.
We could wrap it in a try/except AttributeError perhaps.
|
Let's add to the CHANGELOG as well please |
No description provided.