Page MenuHomePhabricator

Implement caching for ElectronPdfService extension
Closed, DeclinedPublic

Description

If the page was already rendered as PDF before, offer to download the cached PDF and some UI element to force re-rendering.
This requires several things:

  • Implement caching logic in the backend.
  • Add a new "Force re-rendering" UI element to the selection screen (old mockup without such an element: https://phabricator.wikimedia.org/T142201#2660206).
  • In cases where we at the moment are directly linking to the PDF, we need to show a page with a "Download" and a "Force re-rendering" element instead. This needs a design first.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 25 2016, 12:11 PM

The REST API generally uses normal Varnish CDN caching. If long-term caching is needed, then we can also actively purge this end point. Explicit re-rendering should ideally not be needed, but could be supported via the cache-control header.

I would definitely recommend to avoid implementing your own caching infrastructure outside of CDN caching.

Addshore moved this task from Incoming to Tables in pdfs on the TCB-Team board.Oct 25 2016, 2:22 PM

@Tobi_WMDE_SW What is the user story behind a forced rerender? Why would
somebody want to do this? If it's very rare or unique situation, it might
warrant balancing against simplicity for everyone else.

Varnish caching with purging seems like the easiest option. The purging strategy will be the same as for mobile-sections and summary endpoints in RESTBase, so all the infrastructure for purging is already implemented in ChangeProp. Enabling it is a matter of 5-lines config rule.

@Pchelolo, the main caveat with active purging is the extra purge volume. We might want to hold off on setting that up for PDF renders until we can leverage XKey to avoid increasing the purge volume. In the short term, a relatively short TTL should be good enough to provide decent performance and throughput for the relatively low request volumes we see with OCG (~2/s).

@GWicke We're talking about purges on HTML content change, the rate is quite low ~20/s, so that shouldn't be a huge problem..

@JKatzWMF I was coming up with this because when looking into how it is currently done in the Collection extension I thought we would need something similar for Electron. But after @GWicke's answer I think there's no need in implementing some caching logic in the extension.

@Tobi_WMDE_SW Great, thanks for clarifying!

Tobi_WMDE_SW closed this task as Declined.Oct 28 2016, 2:13 PM

Seems that we don't need to add a separate caching mechanism in the extension and we don't need UI for triggering forced re-rendering, so I'm closing it. If it happens we need anything from this after all, feel free to re-open the ticket!

Tobi_WMDE_SW moved this task from Demoed to Done on the WMDE-QWERTY-Team board.
Tobi_WMDE_SW moved this task from Done to Demoed on the WMDE-QWERTY-Team board.Nov 1 2016, 3:10 PM