Allow ESI tie in [#651902]

ESI (supported by Varnish and Akamai and with a slightly different syntax by nginx) allows for HTML tag-like commands in the page source like <esi:include src="http://example.com/ssi.php?key=abcd" > so that the static parts of the page is cached in (for example) Varnish and only the dynamic bits are fetched dynamically. ssi.php is a very small program that connects to, say, memcached reads the value of the specified key and prints it (this esi.php is completely David Strauss' idea). Now, if Drupal would be able to serve page templates and cache the dynamic pieces into memcached then we could scale through the roof. And we are close... patch to follow I just wanted to file the issue so I can link to it from the code comment.

Comment	File	Size	Author
#13	651902-support-esi_13.patch	1.72 KB	scor
#13
#11	651902-support-esi.patch	1.73 KB	chx
#11
#4	651902-support-esi.patch	1.64 KB	Damien Tournoud
#4
#1	esi.patch	1.02 KB	chx
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

chx CreditAttribution: chx commented 5 December 2009 at 08:44

File	Size
esi.patch	1.02 KB

Note that there are no cache implementations that return strings on cache_set so this is a completely harmless addition to core. The speed penalty is extremely minimal: one if and that's a simple if (and cache_set is a relatively rare operation). Once this is in we can add a few lines to say memcache.inc to return the ESI command when adequate (like using a cache_esi bin). (Yes, memcache is ported to D7, with tests)

Comment #2

Dries CreditAttribution: Dries commented 5 December 2009 at 15:48

Interesting, but the code comments are lacking.

Comment #3

catch

he/him

English

CreditAttribution: catch commented 5 December 2009 at 17:23

Issue tags:

+Performance

joshk has been working on something similar to this with Panels in D6 apparently, not sure where the patch lives though. In other words subscribing.

Comment #4

Damien Tournoud CreditAttribution: Damien Tournoud commented 5 December 2009 at 18:12

File	Size
651902-support-esi.patch	1.64 KB

I think this is actually enough to support ESI properly. It's still a little bit hackish (the only true, non hackish, solution would be to make drupal_render_cache_(get|set)() pluggeable... but that would be a significant API change), but it looks nicer I believe.

Needs review

» Reviewed & tested by the community

I like it too. Good to go, IMO

Comment #8

pwolanin CreditAttribution: pwolanin commented 6 December 2009 at 15:39

the change to function drupal_render() looks like a minor optimization unrelated to ESI support?

Comment #9

Dries CreditAttribution: Dries commented 6 December 2009 at 16:26

+++ includes/common.inc
@@ -5203,13 +5204,15 @@ function drupal_render_cache_get($elements) {
+  // Cache implementations are allowed to modify the markup, to support
+  // edge-side replacement of the cached markup.
+  $data['#markup'] = &$markup;

Looks ready to me, but I'd like to see this code comment to be expanded a bit more. Let's add one or two more sentences to be a bit more concrete.