[Varnish] #268: Make 304 and Last-Modified work with ESI

[Varnish]

#268: Make 304 and Last-Modified work with ESI
-------------------------+--------------------------------------------------
 Reporter:  sky          |       Owner:  phk  
     Type:  enhancement  |      Status:  new  
 Priority:  normal       |   Milestone:       
Component:  varnishd     |     Version:  trunk
 Severity:  normal       |    Keywords:       
-------------------------+--------------------------------------------------
 This keeps track of a last_modified for a objheader which is set to the
 most recent last_modified, if that is set then we use that instead of our
 top-level.

 If we get an IMS and we don't have the cached value we return 200,
 otherwise we return according to normal rules. There is a race here if a
 fragment gets updated then IMS won't pick it up. However a normal GET will
 pick it up, update the cached value and any future IMS will be correct. It
 is assumed based on my requests that this is a sufficiently small window
 it is fine. Longer term solutions should be looked at.

 {{{
 ===================================================================
 --- bin/varnishd/cache.h        (revision 2877)
 +++ bin/varnishd/cache.h        (working copy)
 @@ -296,6 +296,9 @@
         char                    *hash;
         unsigned                hashlen;
         VTAILQ_HEAD(, sess)     waitinglist;
 +
 +        /* cached value for ESI */
 +       double                  last_modified_cached;
  };

  /* -------------------------------------------------------------------*/
 @@ -371,6 +374,10 @@
         unsigned                ihashptr;
         unsigned                lhashptr;
         const char              **hashptr;
 +
 +       /* used during ESI processing to figure out the real LM */
 +       double                  last_modified_cached;
 +
  };

  /* -------------------------------------------------------------------
 Index: bin/varnishd/cache_response.c
 ===================================================================
 --- bin/varnishd/cache_response.c       (revision 2877)
 +++ bin/varnishd/cache_response.c       (working copy)
 @@ -56,8 +56,17 @@
         http_PrintfHeader(sp->wrk, sp->fd, sp->http, "Date: %s", lm);
         http_SetHeader(sp->wrk, sp->fd, sp->http, "Via: 1.1 varnish");
         http_PrintfHeader(sp->wrk, sp->fd, sp->http, "X-Varnish: %u",
 sp->xid);
 -       TIM_format(sp->obj->last_modified, lm);
 +
 +       /* Check to do the right thing with regards to ESI */
 +       if(!VTAILQ_EMPTY(&sp->obj->esibits)) {
 +               TIM_format(sp->obj->objhead->last_modified_cached, lm);
 +       } else {
 +               TIM_format(sp->obj->last_modified, lm);
 +       }
 +
         http_PrintfHeader(sp->wrk, sp->fd, sp->http, "Last-Modified: %s",
 lm);
 +
 +
         http_PrintfHeader(sp->wrk, sp->fd, sp->http, "Connection: %s",
             sp->doclose ? "close" : "keep-alive");
         sp->wantbody = 0;
 @@ -70,13 +79,24 @@
  {
         char *p;
         double ims;
 +       double last_modified = 0;

 -       if (sp->obj->last_modified > 0 &&
 +       /* Check to do the right thing with regards to ESI */
 +       if(!VTAILQ_EMPTY(&sp->obj->esibits)) {
 +               last_modified = sp->obj->objhead->last_modified_cached;
 +       } else {
 +               last_modified = sp->obj->last_modified;
 +       }
 +
 +
 +       if (last_modified > 0 &&
             http_GetHdr(sp->http, H_If_Modified_Since, &p)) {
 +
 +
                 ims = TIM_parse(p);
                 if (ims > sp->t_req)    /* [RFC2616 14.25] */
                         return (0);
 -               if (sp->obj->last_modified > ims) {
 +               if (last_modified > ims) {
                         return (0);
                 }
                 res_do_304(sp);
 @@ -134,16 +154,54 @@
         CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);

         WRK_Reset(sp->wrk, &sp->fd);
 -       if (sp->esis == 0)
 -               sp->wrk->acct.hdrbytes += http_Write(sp->wrk, sp->http,
 1);

         if (sp->wantbody && !VTAILQ_EMPTY(&sp->obj->esibits)) {
 +
 +               /* slight duplicate code path to below
 +                  but we need to mangle headers a bit */
 +
 +
 +               if(sp->obj->last_modified <
 sp->obj->objhead->last_modified_cached) {
 +                       char lm[64];
 +                       http_Unset(sp->http, H_Last_Modified);
 +                       TIM_format(sp->obj->objhead->last_modified_cached,
 lm);
 +                       http_PrintfHeader(sp->wrk, sp->fd, sp->http,
 "Last-Modified: %s", lm);
 +
 +               }
 +
 +               if (sp->esis == 0)
 +                       sp->wrk->acct.hdrbytes += http_Write(sp->wrk,
 sp->http, 1);
 +
                 ESI_Deliver(sp);
 +
 +
 +
 +
 +               /* do i really need this mutex?
 +                we have a ref to the object
 +               */
 +               if (sp->esis == 0) {
 +                       if (sp->obj->last_modified >
 sp->last_modified_cached)
 +                               sp->last_modified_cached =
 sp->obj->last_modified;
 +                       LOCK(&sp->obj->objhead->mtx);
 +                       if (sp->last_modified_cached >
 sp->obj->objhead->last_modified_cached)
 +                               sp->obj->objhead->last_modified_cached =
 sp->last_modified_cached;
 +                       UNLOCK(&sp->obj->objhead->mtx);
 +               }
         } else if (sp->wantbody) {
 +
 +               if (sp->esis == 0)
 +                       sp->wrk->acct.hdrbytes += http_Write(sp->wrk,
 sp->http, 1);
 +
                 if (sp->esis > 0) {
                         sprintf(lenbuf, "%x\r\n", sp->obj->len);
                         sp->wrk->acct.hdrbytes +=
                             WRK_Write(sp->wrk, lenbuf, -1);
 +
 +                       /* Try and find the latest last-modified */
 +                       if(sp->obj && sp->obj->last_modified >
 sp->last_modified_cached)
 +                               sp->last_modified_cached =
 sp->obj->last_modified;
 +
                 }

                 VTAILQ_FOREACH(st, &sp->obj->store, list) {
 @@ -172,7 +230,11 @@
                 spassert(u == sp->obj->len);
                 if (sp->esis > 0)
                         WRK_Write(sp->wrk, "\r\n", -1);
 +       } else {
 +               if (sp->esis == 0)
 +                       sp->wrk->acct.hdrbytes += http_Write(sp->wrk,
 sp->http, 1);
         }
 +
         if (WRK_Flush(sp->wrk))
                 vca_close_session(sp, "remote closed");
  }

 }}}

-- 
Ticket URL: <http://varnish.projects.linpro.no/ticket/268>
Varnish <http://varnish.projects.linpro.no/>
The Varnish HTTP Accelerator