You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2015/05/21 02:14:51 UTC

trafficserver git commit: Fixes to partial cached hull logic. Working for most basic test cases.

Repository: trafficserver
Updated Branches:
  refs/heads/ts-974-5-3-x a7b775e20 -> d5a3df4a9


Fixes to partial cached hull logic.
Working for most basic test cases.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/d5a3df4a
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/d5a3df4a
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/d5a3df4a

Branch: refs/heads/ts-974-5-3-x
Commit: d5a3df4a9a053711ad106b64dc0d28fb38d624ab
Parents: a7b775e
Author: Alan M. Carroll <so...@yahoo-inc.com>
Authored: Wed May 20 19:12:59 2015 -0500
Committer: Alan M. Carroll <so...@yahoo-inc.com>
Committed: Wed May 20 19:12:59 2015 -0500

----------------------------------------------------------------------
 iocore/cache/CacheDir.cc   |  2 +-
 iocore/cache/CacheWrite.cc | 17 ++++++++-----
 proxy/hdrs/HTTP.cc         | 54 ++++++++++++++++++++++++-----------------
 proxy/hdrs/HTTP.h          | 21 ++++++++++------
 proxy/http/HttpSM.cc       | 12 +++++----
 5 files changed, 65 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d5a3df4a/iocore/cache/CacheDir.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheDir.cc b/iocore/cache/CacheDir.cc
index 6eedc46..fbd23bf 100644
--- a/iocore/cache/CacheDir.cc
+++ b/iocore/cache/CacheDir.cc
@@ -125,7 +125,7 @@ void
 OpenDir::close_entry(CacheVC* vc)
 {
   ink_assert(vc->vol->mutex->thread_holding == this_ethread());
-  if (--(vc->od->num_active) < 1) {
+  if (vc->od && --(vc->od->num_active) < 1) {
     unsigned int h = vc->od->first_key.slice32(0);
     int b = h % OPEN_DIR_BUCKETS;
     bucket[b].remove(vc->od);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d5a3df4a/iocore/cache/CacheWrite.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheWrite.cc b/iocore/cache/CacheWrite.cc
index 20d6bdb..9901e5f 100644
--- a/iocore/cache/CacheWrite.cc
+++ b/iocore/cache/CacheWrite.cc
@@ -1418,8 +1418,11 @@ CacheVC::openWriteInit(int eid, Event* event)
     }
   }
 
-  if (resp_range.hasRanges()) resp_range.start();
-//  write_pos = resp_range.getOffset();
+  if (resp_range.hasRanges()) {
+    resp_range.start();
+//    this->updateWriteStateFromRange();
+  }
+    
 //  key = alternate.get_frag_key_of(write_pos);
   SET_HANDLER(&CacheVC::openWriteMain);
   return openWriteMain(eid, event);
@@ -1456,7 +1459,7 @@ Lagain:
   int64_t towrite = avail + length;
   int64_t ffs = cacheProcessor.get_fixed_fragment_size();
 
-  Debug("amc", "[CacheVC::openWriteMain] ntodo=%" PRId64 " avail=%" PRId64 " towrite=%" PRId64, ntodo, avail, towrite);
+  Debug("amc", "[CacheVC::openWriteMain] ntodo=%" PRId64 " avail=%" PRId64 " towrite=%" PRId64 " frag=%d", ntodo, avail, towrite, fragment);
 
   if (towrite > ntodo) {
     avail -= (towrite - ntodo);
@@ -1496,17 +1499,19 @@ Lagain:
   if (not_writing)
     return EVENT_CONT;
 
-  ink_assert(static_cast<int64_t>(write_pos) == alternate.get_frag_offset(fragment));
+  this->updateWriteStateFromRange();
+
   {
-    CacheHTTPInfo *alt = 0;
+    CacheHTTPInfo *alt = &alternate;
     MUTEX_LOCK(lock, od->mutex, this_ethread());
 
-    this->updateWriteStateFromRange();
+# if 0
     alternate_index = get_alternate_index(write_vector, earliest_key);
     if (alternate_index < 0)
       alternate_index = write_vector->insert(&alternate, alternate_index);
 
     alt = write_vector->get(alternate_index);
+# endif
 
     if (fragment != 0 && !alt->m_alt->m_earliest.m_flag.cached_p) {
       SET_HANDLER(&CacheVC::openWriteEmptyEarliestDone);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d5a3df4a/proxy/hdrs/HTTP.cc
----------------------------------------------------------------------
diff --git a/proxy/hdrs/HTTP.cc b/proxy/hdrs/HTTP.cc
index 3ead704..a3b4cef 100644
--- a/proxy/hdrs/HTTP.cc
+++ b/proxy/hdrs/HTTP.cc
@@ -2145,7 +2145,8 @@ HTTPInfo::force_frag_at(unsigned int idx) {
       key = frag->m_key;
       old_size = FragmentDescriptorTable::calc_size(old_count);
       memcpy(m_alt->m_fragments, old_table, old_size);
-      ats_free(old_table);
+      if (m_alt->m_flag.table_allocated_p)
+        ats_free(old_table);
     } else {
       key = m_alt->m_earliest.m_key;
       m_alt->m_fragments->m_cached_idx = 0;
@@ -2154,7 +2155,7 @@ HTTPInfo::force_frag_at(unsigned int idx) {
     m_alt->m_flag.table_allocated_p = true;
     // fill out the new parts with offsets & keys.
     ++old_count; // left as the index of the last frag in the previous set.
-    for ( frag = &((*m_alt->m_fragments)[old_count]) ; old_count < n ; ++old_count, ++frag ) {
+    for ( frag = &((*m_alt->m_fragments)[old_count]) ; old_count <= n ; ++old_count, ++frag ) {
       key.next();
       offset += ff_size;
       frag->m_key = key;
@@ -2195,28 +2196,33 @@ int
 HTTPInfo::get_frag_index_of(int64_t offset)
 {
   int zret = 0;
-  int n = this->get_frag_count();
   uint32_t ff_size = this->get_frag_fixed_size();
-
-  if (n < 2) {
-    // Not multi-frag, so just compute the index.
+  FragmentDescriptorTable* table = this->get_frag_table();
+  if (!table) {
+    // Never the case that we have an empty earliest fragment *and* no frag table.
     zret = offset / ff_size;
   } else {
-    FragmentDescriptorTable* frags = this->get_frag_table();
-    zret = (offset / ff_size) + 1;
-    if ((*frags)[1].m_offset == 0) ++zret;
-    else if (static_cast<uint64_t>(offset) >= (*frags)[1].m_offset) { // otherwise it's in the earliest frag, index 0
+    FragmentDescriptorTable& frags = *table; // easier to work with.
+    int n = frags.m_n; // also the max valid frag table index and always >= 1.
+    // I should probably make @a m_offset int64_t to avoid casting issues like this...
+    uint64_t uoffset = static_cast<uint64_t>(offset);
+
+    if (uoffset >= frags[n].m_offset) {
+      // in or past the last fragment, compute the index by computing the # of @a ff_size chunks past the end.
+      zret = n + (static_cast<uint64_t>(offset) - frags[n].m_offset) / ff_size;
+    } else if (uoffset < frags[1].m_offset) {
+      zret = 0; // in the earliest fragment.
+    } else {
       // Need to handle old data where the offsets are not guaranteed to be regular.
-      // So we guess (which should be close) and if we're right, boom, else linear
-      // search.
-      while (0 < zret) {
-        if (static_cast<uint64_t>(offset) < (*frags)[zret].m_offset) {
+      // So we start with our guess (which should be close) and if we're right, boom, else linear
+      // search which should only be 1 or 2 steps.
+      zret = offset / ff_size;
+      if (frags[1].m_offset == 0 || 0 == zret) // zret can be zero if the earliest frag is less than @a ff_size
+        ++zret;
+      while (0 < zret && zret < n) {
+        if (uoffset < frags[zret].m_offset) {
           --zret;
-        } else if (zret >= n-1) {
-          // if we're at the last known fragment, just compute the index.
-          zret += (static_cast<uint64_t>(offset) - (*frags)[zret].m_offset) / ff_size;
-          break;
-        } else if (static_cast<uint64_t>(offset) >= (*frags)[zret+1].m_offset) {
+        } else if (uoffset >= frags[zret+1].m_offset) {
           ++zret;
         } else {
           break;
@@ -2632,21 +2638,25 @@ HTTPInfo::get_uncached_hull(HTTPRangeSpec const& req)
   if (m_alt && !m_alt->m_flag.complete_p) {
     HTTPRangeSpec::Range s = req.getConvexHull();
     if (m_alt->m_fragments) {
+      FragmentDescriptorTable& fdt = *(m_alt->m_fragments);
       int32_t lidx;
       int32_t ridx;
       if (s.isValid()) {
         lidx = this->get_frag_index_of(s._min);
         ridx = this->get_frag_index_of(s._max);
       } else { // not a range request, get hull of all uncached fragments
-        lidx = m_alt->m_fragments->m_cached_idx + 1;
+        lidx = fdt.m_cached_idx + 1;
         // This really isn't valid if !content_length_p, need to deal with that at some point.
         ridx = this->get_frag_index_of(this->object_size_get());
       }
 
       if (lidx < 2 && !m_alt->m_earliest.m_flag.cached_p) lidx = 0;
-      else while (lidx <= ridx && (*m_alt->m_fragments)[lidx].m_flag.cached_p) ++lidx;
+      else {
+        if (0 == lidx) ++lidx; // because if we get here with lidx == 0, earliest is cached and we should skip ahead.
+        while (lidx <= ridx && fdt[lidx].m_flag.cached_p) ++lidx;
+      }
 
-      while (lidx <= ridx && (*m_alt->m_fragments)[ridx].m_flag.cached_p) --ridx;
+      while (lidx <= ridx && fdt[ridx].m_flag.cached_p) --ridx;
 
       if (lidx <= ridx) r = this->get_range_for_frags(lidx, ridx);
     } else { // no fragments past earliest cached yet

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d5a3df4a/proxy/hdrs/HTTP.h
----------------------------------------------------------------------
diff --git a/proxy/hdrs/HTTP.h b/proxy/hdrs/HTTP.h
index c892dc8..a892d8b 100644
--- a/proxy/hdrs/HTTP.h
+++ b/proxy/hdrs/HTTP.h
@@ -1587,7 +1587,12 @@ struct HTTPCacheAlt {
       m_size value) only the descriptors.
   */
   struct FragmentDescriptorTable {
-    uint32_t m_n; ///< The allocated number of entries in the table.
+    /** The number of entries in the table.
+	Because this is a 1 based array, this is also the largest valid index.
+	@note It is 1 less than the total number of fragment descriptors because earliest is stored
+	directly and not in this table.
+     */
+    uint32_t m_n;
 
     /** Fragment index of last initial segment cached.
 
@@ -1600,7 +1605,12 @@ struct HTTPCacheAlt {
     */
     uint32_t m_cached_idx;
 
-    /// 1 based array operator.
+    /** Array operator for fragments in the table (1-based).
+	This is a bit tricky. The earliest fragment is special and so is @b not stored in this table.
+	To make that easier to deal with this array is one based so the containing object can simply
+	pass the index on if it's not 0 (earliest). From an external point of view the array of fragments
+	is zero based.
+     */
     FragmentDescriptor &operator[](int idx);
     /// Calculate the allocation size needed for a maximum array index of @a n.
     static size_t calc_size(uint32_t n);
@@ -1644,7 +1654,7 @@ struct HTTPCacheAlt {
   /// This can be zero for a resident alternate.
   /// @internal In practice this is the high water mark for cached fragments.
   /// Contrast with the @a m_cached_idx in the fragment table - that marks the high
-  /// water of continguously cached fragments.
+  /// water of contiguously cached fragments.
   uint32_t m_frag_count;
 
   /** The target size for fragments in this alternate.
@@ -2162,10 +2172,7 @@ inline HTTPCacheAlt::FragmentDescriptor &HTTPCacheAlt::FragmentDescriptorTable::
 inline size_t
 HTTPCacheAlt::FragmentDescriptorTable::calc_size(uint32_t n)
 {
-  /* Get storage for @a n fragments. Because we store the earliest in the base structure, we only need
-     @a n - 1 entries in the table.
-  */
-  return n < 1 ? 0 : sizeof(FragmentDescriptorTable) + (n - 1) * sizeof(FragmentDescriptor);
+  return n < 1 ? 0 : sizeof(FragmentDescriptorTable) + n * sizeof(FragmentDescriptor);
 }
 
 #if 0

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d5a3df4a/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index f22bfa1..9826283 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1477,6 +1477,8 @@ HttpSM::state_api_callout(int event, void *data)
 void
 HttpSM::handle_api_return()
 {
+  HttpTunnelProducer* p = 0; // used as a scratch var in various cases.
+
   switch (t_state.api_next_action) {
   case HttpTransact::SM_ACTION_API_SM_START:
     if (t_state.client_info.port_attribute == HttpProxyPort::TRANSPORT_BLIND_TUNNEL) {
@@ -1523,12 +1525,11 @@ HttpSM::handle_api_return()
   }
 
   switch (t_state.next_action) {
-  case HttpTransact::SM_ACTION_TRANSFORM_READ: {
-    HttpTunnelProducer *p = setup_transfer_from_transform();
+  case HttpTransact::SM_ACTION_TRANSFORM_READ:
+    p = setup_transfer_from_transform();
     perform_transform_cache_write_action();
     tunnel.tunnel_run(p);
     break;
-  }
   case HttpTransact::SM_ACTION_SERVER_READ: {
     if (unlikely(t_state.did_upgrade_succeed)) {
       // We've sucessfully handled the upgrade, let's now setup
@@ -1553,14 +1554,15 @@ HttpSM::handle_api_return()
         pending_action = cache_sm.open_partial_read(&t_state.hdr_info.client_request);
         cache_sm.cache_write_vc = NULL;
       } else {
-        setup_server_transfer();
+        p = setup_server_transfer();
         perform_cache_write_action();
+        tunnel.tunnel_run(p);
       }
     }
     break;
   }
   case HttpTransact::SM_ACTION_SERVE_FROM_CACHE: {
-    HttpTunnelProducer *p = setup_cache_read_transfer();
+    p = setup_cache_read_transfer();
     tunnel.tunnel_run(p);
     break;
   }