--------------------- PatchSet 8919 Date: 2007/02/14 14:18:20 Author: adri Branch: storework Tag: (none) Log: Begin cutting out the headers from the store memory. There's plenty of bad hacks in here, including: * removing reply->hdr_sz, as we currently don't "know" the size; and almost every use can be safely removed * made objectLen() same as contentLen(), even though they shouldn't be at the moment. Most of the uses of objectLen() is the store manager and these need to be revisited later on * haven't tested range requests at all! * a few fatal()s were introduced into code paths which need to be reverified and fixed - notably the 304 reply generation. Shouldn't be hard to fix -that- one as it just involves building replies better. Members: src/HttpReply.c:1.19->1.19.2.1 src/client_side.c:1.168.2.2->1.168.2.3 src/ftp.c:1.38->1.38.2.1 src/mime.c:1.18->1.18.2.1 src/protos.h:1.133.2.2->1.133.2.3 src/store.c:1.45.2.3->1.45.2.4 src/store_client.c:1.25.2.1->1.25.2.2 src/store_log.c:1.11->1.11.2.1 src/structs.h:1.139.2.2->1.139.2.3 Index: squid/src/HttpReply.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/HttpReply.c,v retrieving revision 1.19 retrieving revision 1.19.2.1 diff -u -r1.19 -r1.19.2.1 --- squid/src/HttpReply.c 26 Jan 2007 02:52:43 -0000 1.19 +++ squid/src/HttpReply.c 14 Feb 2007 14:18:20 -0000 1.19.2.1 @@ -1,6 +1,6 @@ /* - * $Id: HttpReply.c,v 1.19 2007/01/26 02:52:43 squidadm Exp $ + * $Id: HttpReply.c,v 1.19.2.1 2007/02/14 14:18:20 adri Exp $ * * DEBUG: section 58 HTTP Reply (Response) * AUTHOR: Alex Rousskov @@ -80,7 +80,7 @@ httpReplyInit(HttpReply * rep) { assert(rep); - rep->hdr_sz = 0; + //rep->hdr_sz = 0; rep->pstate = psReadyToParseStartLine; httpBodyInit(&rep->body); httpHeaderInit(&rep->header, hoReply); @@ -170,32 +170,16 @@ void httpReplySwapOut(HttpReply * rep, StoreEntry * e) { - Packer p; assert(rep && e); + /* We only should be packing reply headers with no object data there! */ + assert(e->mem_obj->inmem_hi == 0); + if (rep != e->mem_obj->reply) { httpReplyAbsorb(e->mem_obj->reply, rep); rep = e->mem_obj->reply; } - packerToStoreInit(&p, e); - httpReplyPackInto(e->mem_obj->reply, &p); - packerClean(&p); - rep->hdr_sz = e->mem_obj->inmem_hi - rep->body.mb.size; -} - -#if UNUSED_CODE -MemBuf -httpPackedReply(http_version_t ver, http_status status, const char *ctype, - squid_off_t clen, time_t lmt, time_t expires) -{ - HttpReply *rep = httpReplyCreate(); - MemBuf mb; - httpReplySetHeaders(rep, ver, status, ctype, NULL, clen, lmt, expires); - mb = httpReplyPack(rep); - httpReplyDestroy(rep); - return mb; } -#endif MemBuf httpPacked304Reply(const HttpReply * rep) @@ -208,6 +192,8 @@ HttpHeaderEntry *e; assert(rep); + fatal("This shouldn't be called!\n"); + memBufDefInit(&mb); packerToMemInit(&p, &mb); memBufPrintf(&mb, "%s", "HTTP/1.0 304 Not Modified\r\n"); @@ -355,7 +341,7 @@ HttpReply *dst = httpReplyCreate(); /* basic variables */ - dst->hdr_sz = src->hdr_sz; + //dst->hdr_sz = src->hdr_sz; dst->content_length = src->content_length; dst->date = src->date; dst->last_modified = src->last_modified; @@ -443,7 +429,7 @@ /* Update rep */ httpReplyHdrCacheInit(rep); /* the previous code had hdr_sz including the status line + headers and final \r\n */ - rep->hdr_sz = parse_start - buf; + //rep->hdr_sz = parse_start - buf; rep->pstate++; /* Done */ Index: squid/src/client_side.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/client_side.c,v retrieving revision 1.168.2.2 retrieving revision 1.168.2.3 diff -u -r1.168.2.2 -r1.168.2.3 --- squid/src/client_side.c 13 Feb 2007 23:18:15 -0000 1.168.2.2 +++ squid/src/client_side.c 14 Feb 2007 14:18:20 -0000 1.168.2.3 @@ -1,6 +1,6 @@ /* - * $Id: client_side.c,v 1.168.2.2 2007/02/13 23:18:15 hno Exp $ + * $Id: client_side.c,v 1.168.2.3 2007/02/14 14:18:20 adri Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -2098,6 +2098,7 @@ return; } if (is_modified == 0) { + fatal("XXX this needs to be updated to set reply headers and not append!\n"); time_t timestamp = e->timestamp; MemBuf mb = httpPacked304Reply(e->mem_obj->reply); http->log_type = LOG_TCP_IMS_HIT; @@ -2431,6 +2432,7 @@ assert(http->out.offset == 0); rep = http->reply = clientBuildReply(http, buf, size); if (!rep) { + fatal("clientSendMoreHeaderData: Shouldn't have happened!\n"); /* Forward as HTTP/0.9 body with no reply */ MemBuf mb; memBufDefInit(&mb); @@ -2440,9 +2442,12 @@ return; } if (Config.onoff.log_mime_hdrs) { + /* XXX this is disabled for now; should probably temporarily use the packer to put headers in */ safe_free(http->al.headers.reply); +#if 0 http->al.headers.reply = xcalloc(rep->hdr_sz + 1, 1); xstrncpy(http->al.headers.reply, buf, rep->hdr_sz); +#endif } clientMaxBodySize(http->request, http, rep); if (http->log_type != LOG_TCP_DENIED && clientReplyBodyTooLarge(http, rep->content_length)) { @@ -2466,7 +2471,8 @@ * and body data, writing them out in one swift hit" logic which I've just disabled. * - [ahc] */ - http->range_iter.prefix_size = rep->hdr_sz; + //http->range_iter.prefix_size = rep->hdr_sz; + http->range_iter.prefix_size = 0; CBDATA_INIT_TYPE(clientCheckHeaderStateData); state = cbdataAlloc(clientCheckHeaderStateData); state->http = http; @@ -2477,8 +2483,9 @@ state->body_buf = NULL; state->body_size = 0; assert(state->body_size >= 0); - debug(33, 3) ("clientSendMoreHeaderData: Appending %d bytes after %d bytes of headers\n", - (int) state->body_size, rep->hdr_sz); + /* XXX should re-instansiate the header length calculation here */ + debug(33, 3) ("clientSendMoreHeaderData: Appending %d bytes of data after headers\n", + (int) state->body_size); clientHttpLocationRewriteCheck(state); } @@ -2647,7 +2654,7 @@ storeLockObject(e); http->sc = storeClientRegister(http->entry, http); /* Adjust the header size */ - state->http->reply->hdr_sz = body_offset; + //state->http->reply->hdr_sz = body_offset; /* Clean up any old body content */ httpBodyClean(&state->http->reply->body); state->body_buf = NULL; @@ -2704,7 +2711,9 @@ mb = httpReplyPack(rep); else memBufDefInit(&mb); - http->out.offset += rep->hdr_sz; + /* headers now start at offset 0; we should hopefully try to append some data here later */ + http->out.offset = 0; + http->flags.written_headers = 0; #if HEADERS_LOG headersLog(0, 0, http->request->method, rep); #endif @@ -2884,12 +2893,18 @@ int done; http->out.size += size; debug(33, 5) ("clientWriteComplete: FD %d, sz %d, err %d, off %" PRINTF_OFF_T ", len %" PRINTF_OFF_T "\n", - fd, (int) size, errflag, http->out.offset, entry ? objectLen(entry) : (squid_off_t) 0); + fd, (int) size, errflag, http->out.offset, entry ? contentLen(entry) : (squid_off_t) 0); if (size > 0) { kb_incr(&statCounter.client_http.kbytes_out, size); if (isTcpHit(http->log_type)) kb_incr(&statCounter.client_http.hit_kbytes_out, size); } + /* Did we write headers? Can we assume here we wrote them all? */ + if (http->flags.written_headers == 0) { + http->flags.written_headers = 1; + debug(33, 1) ("clientWriteComplete: FD: written headers; size was %d\n", fd); + } + #if SIZEOF_SQUID_OFF_T <= 4 if (http->out.size > 0x7FFF0000) { debug(33, 1) ("WARNING: closing FD %d to prevent counter overflow\n", fd); @@ -4482,7 +4497,6 @@ StoreEntry *entry = http->entry; MemObject *mem; http_reply *reply; - squid_off_t sendlen; if (entry == NULL) return 0; /* @@ -4496,7 +4510,7 @@ * objectLen(entry) will be set proprely. */ if (entry->store_status == STORE_OK) { - if (http->out.offset >= objectLen(entry)) + if (http->out.offset >= contentLen(entry)) return 1; else return 0; @@ -4508,7 +4522,7 @@ assert(mem != NULL); assert(http->request != NULL); reply = mem->reply; - if (reply->hdr_sz == 0) + if (reply->pstate < psParsed) return 0; /* haven't found end of headers yet */ else if (reply->sline.status == HTTP_OK) sending = SENDING_BODY; @@ -4527,29 +4541,27 @@ * If we are sending a body and we don't have a content-length, * then we must wait for the object to become STORE_OK. */ - if (sending == SENDING_HDRSONLY) - sendlen = reply->hdr_sz; - else if (reply->content_length < 0) - return 0; - else - sendlen = reply->content_length + reply->hdr_sz; - /* - * Now that we have the expected length, did we send it all? - */ - if (http->out.offset < sendlen) - return 0; - else - return 1; + /* body? Check content length and return accordingly */ + if (sending == SENDING_BODY && reply->content_length < 0) { + return 0; /* can't know if we've sent enough */ + } else if (sending == SENDING_BODY) { + return(http->out.offset >= reply->content_length); + } + + /* Headers only here; have we written headers? */ + if (sending == SENDING_HDRSONLY) { + return http->flags.written_headers; + } + fatal("clientCheckTransferDone: shouldn't get here\n"); } static int clientGotNotEnough(clientHttpRequest * http) { squid_off_t cl = httpReplyBodySize(http->request->method, http->entry->mem_obj->reply); - int hs = http->entry->mem_obj->reply->hdr_sz; if (cl < 0) return 0; - if (http->out.offset != cl + hs) + if (http->out.offset != cl) return 1; return 0; } Index: squid/src/ftp.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/ftp.c,v retrieving revision 1.38 retrieving revision 1.38.2.1 diff -u -r1.38 -r1.38.2.1 --- squid/src/ftp.c 19 Jan 2007 01:15:53 -0000 1.38 +++ squid/src/ftp.c 14 Feb 2007 14:18:23 -0000 1.38.2.1 @@ -1,6 +1,6 @@ /* - * $Id: ftp.c,v 1.38 2007/01/19 01:15:53 squidadm Exp $ + * $Id: ftp.c,v 1.38.2.1 2007/02/14 14:18:23 adri Exp $ * * DEBUG: section 9 File Transfer Protocol (FTP) * AUTHOR: Harvest Derived @@ -2594,7 +2594,7 @@ if (mime_enc) httpHeaderPutStr(&reply->header, HDR_CONTENT_ENCODING, mime_enc); httpReplySwapOut(reply, e); - reply->hdr_sz = e->mem_obj->inmem_hi; + //reply->hdr_sz = e->mem_obj->inmem_hi; storeTimestampsSet(e); if (ftpState->flags.authenticated) { /* Index: squid/src/mime.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/mime.c,v retrieving revision 1.18 retrieving revision 1.18.2.1 diff -u -r1.18 -r1.18.2.1 --- squid/src/mime.c 21 Jan 2007 14:03:54 -0000 1.18 +++ squid/src/mime.c 14 Feb 2007 14:18:24 -0000 1.18.2.1 @@ -1,6 +1,6 @@ /* - * $Id: mime.c,v 1.18 2007/01/21 14:03:54 squidadm Exp $ + * $Id: mime.c,v 1.18.2.1 2007/02/14 14:18:24 adri Exp $ * * DEBUG: section 25 MIME Parsing * AUTHOR: Harvest Derived @@ -444,7 +444,7 @@ httpHdrCcSetMaxAge(reply->cache_control, 86400); httpHeaderPutCc(&reply->header, reply->cache_control); httpReplySwapOut(reply, e); - reply->hdr_sz = e->mem_obj->inmem_hi; /* yuk */ + //reply->hdr_sz = e->mem_obj->inmem_hi; /* yuk */ /* read the file into the buffer and append it to store */ buf = memAllocate(MEM_4K_BUF); while ((n = FD_READ_METHOD(fd, buf, 4096)) > 0) Index: squid/src/protos.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/protos.h,v retrieving revision 1.133.2.2 retrieving revision 1.133.2.3 diff -u -r1.133.2.2 -r1.133.2.3 --- squid/src/protos.h 13 Feb 2007 10:47:34 -0000 1.133.2.2 +++ squid/src/protos.h 14 Feb 2007 14:18:24 -0000 1.133.2.3 @@ -1,6 +1,6 @@ /* - * $Id: protos.h,v 1.133.2.2 2007/02/13 10:47:34 adri Exp $ + * $Id: protos.h,v 1.133.2.3 2007/02/14 14:18:24 adri Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -1076,6 +1076,7 @@ */ extern store_client *storeClientRegister(StoreEntry * e, void *data); extern void storeClientCopy(store_client *, StoreEntry *, squid_off_t, squid_off_t, size_t, char *, STCB *, void *); +extern void storeClientCopyData(store_client *, StoreEntry *, squid_off_t, squid_off_t, size_t, char *, STCB *, void *); extern int storeClientCopyPending(store_client *, StoreEntry * e, void *data); extern int storeClientUnregister(store_client * sc, StoreEntry * e, void *data); extern squid_off_t storeLowestMemReaderOffset(const StoreEntry * entry); Index: squid/src/store.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/store.c,v retrieving revision 1.45.2.3 retrieving revision 1.45.2.4 diff -u -r1.45.2.3 -r1.45.2.4 --- squid/src/store.c 13 Feb 2007 23:17:12 -0000 1.45.2.3 +++ squid/src/store.c 14 Feb 2007 14:18:25 -0000 1.45.2.4 @@ -1,6 +1,6 @@ /* - * $Id: store.c,v 1.45.2.3 2007/02/13 23:17:12 hno Exp $ + * $Id: store.c,v 1.45.2.4 2007/02/14 14:18:25 adri Exp $ * * DEBUG: section 20 Storage Manager * AUTHOR: Harvest Derived @@ -938,8 +938,8 @@ debug(20, 3) ("storeEntryValidLength: Checking '%s'\n", storeKeyText(e->hash.key)); debug(20, 5) ("storeEntryValidLength: object_len = %" PRINTF_OFF_T "\n", objectLen(e)); - debug(20, 5) ("storeEntryValidLength: hdr_sz = %d\n", - reply->hdr_sz); + //debug(20, 5) ("storeEntryValidLength: hdr_sz = %d\n", +// reply->hdr_sz); clen = httpReplyBodySize(e->mem_obj->method, reply); debug(20, 5) ("storeEntryValidLength: content_length = %" PRINTF_OFF_T "\n", clen); @@ -948,7 +948,7 @@ storeKeyText(e->hash.key)); return 1; } - diff = reply->hdr_sz + clen - objectLen(e); + diff = clen - objectLen(e); if (diff == 0) return 1; debug(20, 2) ("storeEntryValidLength: %" PRINTF_OFF_T " bytes too %s; '%s'\n", @@ -1243,6 +1243,10 @@ } } +/* + * XXX objectLen and contentLen are now the same? Do we need to + * include the header length here too? + */ squid_off_t objectLen(const StoreEntry * e) { @@ -1255,7 +1259,7 @@ { assert(e->mem_obj != NULL); assert(e->mem_obj->reply != NULL); - return e->mem_obj->object_sz - e->mem_obj->reply->hdr_sz; + return e->mem_obj->object_sz; } HttpReply * Index: squid/src/store_client.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/store_client.c,v retrieving revision 1.25.2.1 retrieving revision 1.25.2.2 diff -u -r1.25.2.1 -r1.25.2.2 --- squid/src/store_client.c 11 Feb 2007 09:12:34 -0000 1.25.2.1 +++ squid/src/store_client.c 14 Feb 2007 14:18:26 -0000 1.25.2.2 @@ -1,6 +1,6 @@ /* - * $Id: store_client.c,v 1.25.2.1 2007/02/11 09:12:34 adri Exp $ + * $Id: store_client.c,v 1.25.2.2 2007/02/14 14:18:26 adri Exp $ * * DEBUG: section 20 Storage Manager Client-Side Interface * AUTHOR: Duane Wessels @@ -156,6 +156,53 @@ storeClientCopy2(sc->entry, sc); } +/* + * Copy some data. seen_offset and copy_offset are offset into the + * data, _not_ including the reply. This routine must only be called + * once the header reply+status size is known - so it can't currently + * be called off the bat. The reply + headers must first be read in. + */ +void +storeClientCopyData(store_client *sc, + StoreEntry * e, + squid_off_t seen_offset, + squid_off_t copy_offset, + size_t size, + char *buf, + STCB * callback, + void *data) +{ + assert(!EBIT_TEST(e->flags, ENTRY_ABORTED)); + debug(20, 3) ("storeClientCopy: %s, seen %" PRINTF_OFF_T ", want %" PRINTF_OFF_T ", size %d, cb %p, cbdata %p\n", + storeKeyText(e->hash.key), + seen_offset, + copy_offset, + (int) size, + callback, + data); + assert(sc != NULL); +#if STORE_CLIENT_LIST_DEBUG + assert(sc == storeClientListSearch(e->mem_obj, data)); +#endif + assert(sc->callback == NULL); + assert(sc->entry == e); + sc->seen_offset = seen_offset; + sc->callback = callback; + sc->callback_data = data; + cbdataLock(sc->callback_data); + sc->copy_buf = buf; + sc->copy_size = size; + sc->copy_offset = copy_offset; + /* If the read is being deferred, run swapout in case this client has the + * lowest seen_offset. storeSwapOut() frees the memory and clears the + * ENTRY_DEFER_READ bit if necessary */ + if (EBIT_TEST(e->flags, ENTRY_DEFER_READ)) { + storeSwapOut(e); + } + storeClientCopy2(e, sc); +} + + /* copy bytes requested by the client */ void storeClientCopy(store_client * sc, @@ -212,7 +259,8 @@ squid_off_t len; if (e->store_status == STORE_PENDING) return 0; - if ((len = objectLen(e)) < 0) + /* XXX changed to contentLen - do we need to care about headers too? */ + if ((len = contentLen(e)) < 0) return 0; if (sc->copy_offset < len) return 0; @@ -431,7 +479,7 @@ debug(20, 3) ("CheckQuickAbort2: YES KEY_PRIVATE\n"); return 1; } - expectlen = mem->reply->content_length + mem->reply->hdr_sz; + expectlen = mem->reply->content_length; curlen = mem->inmem_hi; minlen = Config.quickAbort.min << 10; if (minlen < 0) { Index: squid/src/store_log.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/store_log.c,v retrieving revision 1.11 retrieving revision 1.11.2.1 diff -u -r1.11 -r1.11.2.1 --- squid/src/store_log.c 21 Jan 2007 14:04:11 -0000 1.11 +++ squid/src/store_log.c 14 Feb 2007 14:18:26 -0000 1.11.2.1 @@ -1,6 +1,6 @@ /* - * $Id: store_log.c,v 1.11 2007/01/21 14:04:11 squidadm Exp $ + * $Id: store_log.c,v 1.11.2.1 2007/02/14 14:18:26 adri Exp $ * * DEBUG: section 20 Storage Manager Logging Functions * AUTHOR: Duane Wessels @@ -77,7 +77,7 @@ (long int) reply->expires, strLen(reply->content_type) ? strBuf(reply->content_type) : "unknown", reply->content_length, - mem->inmem_hi - mem->reply->hdr_sz, + mem->inmem_hi, RequestMethods[mem->method].str, rfc1738_escape_unescaped(mem->url)); } else { Index: squid/src/structs.h =================================================================== RCS file: /cvsroot/squid-sf//squid/src/structs.h,v retrieving revision 1.139.2.2 retrieving revision 1.139.2.3 diff -u -r1.139.2.2 -r1.139.2.3 --- squid/src/structs.h 13 Feb 2007 23:18:21 -0000 1.139.2.2 +++ squid/src/structs.h 14 Feb 2007 14:18:26 -0000 1.139.2.3 @@ -1,6 +1,6 @@ /* - * $Id: structs.h,v 1.139.2.2 2007/02/13 23:18:21 hno Exp $ + * $Id: structs.h,v 1.139.2.3 2007/02/14 14:18:26 adri Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -1065,7 +1065,7 @@ struct _HttpReply { /* unsupported, writable, may disappear/change in the future */ - int hdr_sz; /* sums _stored_ status-line, headers, and */ + //int hdr_sz; /* sums _stored_ status-line, headers, and */ /* public, readable; never update these or their .hdr equivalents directly */ squid_off_t content_length; @@ -1219,6 +1219,7 @@ aclCheck_t *acl_checklist; /* need ptr back so we can unreg if needed */ AccessLogEntry al; struct { + unsigned int written_headers:1; unsigned int accel:1; unsigned int transparent:1; unsigned int internal:1;