RFC 72: Store routes in content-store only #72
Conversation
|
|
||
| ### Changes required to content-store | ||
|
|
||
| Content-store should start storing routes (including redirects) as separate collections in its own datastore. This could be achieved by copying over the Route and Backend models from router-api. (Although in theory it should be possible to modify router to read the routes and redirects from the existing ContentItem model structure, attempts to do so made route reloading impractically slow.) A migration will be needed to create all the existing routes and backends. |
There was a problem hiding this comment.
By paul.bowsher on 2017-05-11 13:43:19.341
Can you talk more about or link to this experiment? Was this tried in Ruby code or by writing a map/reduce function to run inside Mongo? It *feels* (with absolutely no evidence whatsoever) that writing a query for the JS engine to execute should be able to return things quickly enough. It'd also be good to know how slow is impractical.
There was a problem hiding this comment.
By danielroseman on 2017-05-11 15:05:40.263
I originally did an experiment with rewriting the router app to read from the content_items collection. But the slowness was in the iteration over the data in Mongo itself, not in the Go code; it seems that (at least in the old MongoDB version we are running), doing any kind of iteration over documents that contain large amounts of data, even if those fields are excluded via projection, is slow - I measured it as taking 37 seconds, vs < 5 seconds in the original. My concern was that this would lead to a return of the kind of issues that led us to introduce the async reloading last year.
I don't think mapreduce or aggregation would help; we just need to iterate over a dataset.
But in any case, having rejected this option I now do think we can get a better outcome by splitting out the routes and redirects anyway; it simplifies the data as well as the lookup code.
|
|
||
| Whitehall's organisation\_slug\_changer task is the only other place in our stack that calls router-api directly. This should be rewritten so that it send redirects to the publishing-api. | ||
|
|
||
| All publishing applications should send explicit routes to the publishing platform for documents that contain sub-pages. Travel-advice-publisher is already doing this, but publisher is not. |
There was a problem hiding this comment.
By tijmen.brommet on 2017-05-11 15:37:45.260
What would happen if we didn't do this?
There was a problem hiding this comment.
By danielroseman on 2017-05-11 15:57:21.861
You're right that this may not be required. My thinking was that it would make it hard to find the content item related to a sub-page - but the same argument could be made about any prefix route, and we clearly do need to continue to support those for things like smart-answers and local transactions. But extending the travel-advice change to guides in publisher would probably be a good cleanup nonetheless.
|
|
||
| ### Changes required to content-store | ||
|
|
||
| Content-store should start storing routes (including redirects) as separate collections in its own datastore. This could be achieved by copying over the Route and Backend models from router-api. (Although in theory it should be possible to modify router to read the routes and redirects from the existing ContentItem model structure, attempts to do so made route reloading impractically slow.) A migration will be needed to create all the existing routes and backends. |
There was a problem hiding this comment.
Router currently allows things that router-api doesn't - for example an exact and a prefix route at the same level is allowed in router, but not in router-api
Is the intention to reuse the router-api behaviour, try and get closer to router's capabilities, or something else?
There was a problem hiding this comment.
Do you know the history of that difference? If router-api prevents it, then the only way this can happen is via router-data, which we're also killing anyway.
|
I don't, but I would have found it useful with Trade tariff when we had a
transaction at /trade-tariff and the app at some routes under that (that we
didn't control). I'm not sure of anything else that we have that would
benefit from this approach at the moment.
On 19 May 2017 17:39, "Daniel Roseman" <notifications@github.com> wrote:
*@danielroseman* commented on this pull request.
------------------------------
In rfc-072-store-routes-in-content-store-only.md
<#72 (comment)>:
+
+Because router reads from the datastore rather than the api,
migrating to use content-store should mainly require storing the
information in the appropriate form in the content-store MongoDB - in
other words, removing the `routes` and `redirects` fields
from ContentItem and moving the data into a separate Route model, as
well as creating a Backend model. Router can then be pointed at the
content-store database.
+
+## Implementation
+
+### Preconditions
+
+The work to migrate router-data to short-url-manager needs to be
finished. Some of the files in that repository have not been possible
to migrate, because they contain routes with that conflict with
existing routes in publishing-api that have different publishing apps.
Some time needs to be dedicated to this job.
+
+Whitehall's organisation\_slug\_changer task is the only other place
in our stack that calls router-api directly. This should be rewritten
so that it send redirects to the publishing-api.
+
+All publishing applications should send explicit routes to the
publishing platform for documents that contain sub-pages.
Travel-advice-publisher is already doing this, but publisher is not.
+
+### Changes required to content-store
+
+Content-store should start storing routes (including redirects) as
separate collections in its own datastore. This could be achieved by
copying over the Route and Backend models from router-api. (Although
in theory it should be possible to modify router to read the routes
and redirects from the existing ContentItem model structure, attempts
to do so made route reloading impractically slow.) A migration will be
needed to create all the existing routes and backends.
Do you know the history of that difference? If router-api prevents it, then
the only way this can happen is via router-data, which we're also killing
anyway.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#72 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAvLrXgfHESoH2O059IR44Qcu4zVpqmhks5r7cXHgaJpZM4Nd9ir>
.
|
|
This proposal moves us away from the idea of dumbing down Content Store so it could in theory be just a bunch of files or S3 keys. We've never truly cemented that idea and agreed on it, but we should discuss the implications. |
|
@danielroseman should we close this until there's consensus? |
|
Yeah, probably. I still think it would be a useful simplification on balance, albeit at the cost of adding some extra complexity in content-store. But since there is little prospect of anyone having time to do the work in the near future anyway, we might as well close it. |
No description provided.