trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a..@apache.org
Subject [1/2] trafficserver git commit: Fixed not waiting on fragments in missing frag table.
Date Mon, 04 May 2015 03:27:08 GMT
Repository: trafficserver
Updated Branches:
  refs/heads/ts-974-5-3-x f82fd704f -> 2fb305bb4


Fixed not waiting on fragments in missing frag table.


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

Branch: refs/heads/ts-974-5-3-x
Commit: e92da07f8d2ddd16ad9250ee3ef08d9c1b09f4b7
Parents: f82fd70
Author: Alan M. Carroll <solidwallofcode@yahoo-inc.com>
Authored: Sun May 3 08:48:50 2015 -0500
Committer: Alan M. Carroll <solidwallofcode@yahoo-inc.com>
Committed: Sun May 3 08:48:50 2015 -0500

----------------------------------------------------------------------
 iocore/cache/CacheDir.cc   |  9 +++---
 iocore/cache/CacheHttp.cc  | 14 ++++++---
 iocore/cache/CacheRead.cc  | 68 +++++++++--------------------------------
 iocore/cache/CacheWrite.cc |  4 +--
 iocore/cache/P_CacheDir.h  |  4 ++-
 iocore/cache/P_CacheHttp.h | 12 +++++++-
 proxy/http/HttpCacheSM.cc  |  5 +++
 proxy/http/HttpSM.cc       |  3 ++
 8 files changed, 51 insertions(+), 68 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e92da07f/iocore/cache/CacheDir.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheDir.cc b/iocore/cache/CacheDir.cc
index 9bac5d8..038e6f0 100644
--- a/iocore/cache/CacheDir.cc
+++ b/iocore/cache/CacheDir.cc
@@ -143,7 +143,7 @@ OpenDir::close_write(CacheVC *cont)
     int b = h % OPEN_DIR_BUCKETS;
     bucket[b].remove(cont->od);
 //    delayed_readers.append(cont->od->readers);
-    signal_readers(0, 0);
+//    signal_readers(0, 0);
     cont->od->vector.clear();
     cont->od->mutex = 0;
     THREAD_FREE(cont->od, openDirEntryAllocator, cont->mutex->thread_holding);
@@ -216,12 +216,11 @@ OpenDirEntry::key_for(CacheKey const& alt_key, int64_t offset)
   return vector.key_for(alt_key, offset);
 }
 
-OpenDirEntry&
-OpenDirEntry::waiting_for(CacheKey const& alt_key, CacheVC* vc, int64_t offset)
+bool
+OpenDirEntry::wait_for(CacheKey const& alt_key, CacheVC* vc, int64_t offset)
 {
   Debug("amc", "vc %p waiting for %" PRId64, vc, offset);
-  vector.waiting_for(alt_key, vc, offset);
-  return *this;
+  return vector.wait_for(alt_key, vc, offset);
 }
 
 OpenDirEntry&

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e92da07f/iocore/cache/CacheHttp.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheHttp.cc b/iocore/cache/CacheHttp.cc
index 97837ee..6078d22 100644
--- a/iocore/cache/CacheHttp.cc
+++ b/iocore/cache/CacheHttp.cc
@@ -315,15 +315,19 @@ CacheHTTPInfoVector::is_write_active(CacheKey const& alt_key, int64_t
offset)
 /*-------------------------------------------------------------------------
   -------------------------------------------------------------------------*/
 
-CacheHTTPInfoVector&
-CacheHTTPInfoVector::waiting_for(CacheKey const& alt_key, CacheVC* vc, int64_t offset)
+bool
+CacheHTTPInfoVector::wait_for(CacheKey const& alt_key, CacheVC* vc, int64_t offset)
 {
+  bool zret = true;
   int alt_idx = this->index_of(alt_key);
   Item& item = data[alt_idx];
   int frag_idx = item._alternate.get_frag_index_of(offset);
-  vc->fragment = frag_idx;
-  item._waiting.push(vc);
-  return *this;
+  vc->fragment = frag_idx; // really? Shouldn't this already be set?
+  if (item.has_writers())
+    item._waiting.push(vc);
+  else
+    zret = false;
+  return zret;
 }
 
 /*-------------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e92da07f/iocore/cache/CacheRead.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheRead.cc b/iocore/cache/CacheRead.cc
index 8c49076..726c834 100644
--- a/iocore/cache/CacheRead.cc
+++ b/iocore/cache/CacheRead.cc
@@ -354,6 +354,7 @@ CacheVC::openReadFromWriter(int event, Event *e)
   if (write_vc && CACHE_ALT_INDEX_DEFAULT != (alternate_index = get_alternate_index(&(od->vector),
write_vc->earliest_key))) {
     alternate.copy_shallow(od->vector.get(alternate_index));
     key = earliest_key = alternate.object_key_get();
+    doc_len = alternate.object_size_get();
     MUTEX_RELEASE(lock_od);
     SET_HANDLER(&CacheVC::openReadStartEarliest);
     return openReadStartEarliest(event, e);
@@ -661,36 +662,13 @@ LreadMain:
 void
 CacheVC::update_key_to_frag_idx(int target)
 {
-  FragmentDescriptorTable& frags = (*alternate.get_frag_table());
-
   if (0 == target) {
     fragment = 0;
     key = earliest_key;
-  } else if (! frags[target].m_key.is_zero()) {
-    key = frags[target].m_key;
-  } else { // step through the hard way
-    if (target < fragment) { // going backwards
-      if (target < (fragment - target)) { // faster to go from start
-        fragment = 0;
-        key = earliest_key;
-      } else { // quicker to go from here to there
-        while (target < fragment) {
-          prev_CacheKey(&key, &key);
-          --fragment;
-          if (frags[fragment].m_key.is_zero())
-            frags[fragment].m_key = key; // might as well store it as we go.
-          else ink_assert(frags[fragment].m_key == key);
-        }
-      }
-    }
-    // advance to target if we're not already there.
-    while (target > fragment) {
-      next_CacheKey(&key, &key);
-      ++fragment;
-      if (frags[fragment].m_key.is_zero())
-        frags[fragment].m_key = key; // might as well store it as we go.
-      else ink_assert(frags[fragment].m_key == key);
-    }
+  } else {
+    FragmentDescriptor* frag = alternate.force_frag_at(target);
+    ink_assert(frag);
+    key = frag->m_key;
   }
 }
 
@@ -779,7 +757,7 @@ CacheVC::openReadMain(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED
*/)
     int n_frags = alternate.get_frag_count();
 
     // Quick check for offset in next fragment - very common
-    if (target_offset >= frag_upper_bound && (!frags || fragment > n_frags
|| target_offset < (*frags)[fragment].m_offset)) {
+    if (target_offset >= frag_upper_bound && (!frags || fragment >= n_frags
|| target_offset <= (*frags)[fragment].m_offset)) {
       Debug("amc", "Non-seeking continuation to next fragment");
     } else {
       int target = -1; // target fragment index.
@@ -824,32 +802,16 @@ CacheVC::openReadMain(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED
*/)
       return handleEvent(AIO_EVENT_DONE, 0);
     }
     return EVENT_CONT;
-  } else if (write_vc) {
-    ink_release_assert(! "[amc] Handle case where fragment is not in the directory during
read");
-    /* Check for an OpenDirEntry - if none, no hope. If so, see if any of the writers in
it are still
-       planning to write this fragment (might check against all fragments - if any aren't
going to
-       be written, might just give up at that point rather than hope some other user agent
will happen
-       to ask for the missing fragments. Although, if there's a herd, that might not be so
far fetched).
-    */
-# if 0
-    if (writer_done()) {
-      last_collision = NULL;
-      while (dir_probe(&earliest_key, vol, &dir, &last_collision)) {
-        if (dir_offset(&dir) == dir_offset(&earliest_dir)) {
-          DDebug("cache_read_agg", "%p: key: %X ReadMain complete: %d",
-                 this, first_key.slice32(1), (int)vio.ndone);
-          doc_len = vio.ndone;
-          goto Leos;
-        }
-      }
+  } else {
+    if (!od->wait_for(earliest_key, this, target_offset)) {
       DDebug("cache_read_agg", "%p: key: %X ReadMain writer aborted: %d",
              this, first_key.slice32(1), (int)vio.ndone);
-      goto Lerror;
+      lock.release();
+      return calluser(VC_EVENT_ERROR);
     }
-    DDebug("cache_read_agg", "%p: key: %X ReadMain retrying: %d", this, first_key.slice32(1),
(int)vio.ndone);
+    DDebug("cache_read_agg", "%p: key: %X ReadMain waiting: %d", this, first_key.slice32(1),
(int)vio.ndone);
     SET_HANDLER(&CacheVC::openReadMain);
-    VC_SCHED_WRITER_RETRY();
-# endif
+    return EVENT_CONT;
   }
   if (is_action_tag_set("cache"))
     ink_release_assert(false);
@@ -883,9 +845,7 @@ CacheVC::openReadWaitEarliest(int evid, Event*)
     od = NULL;
     key = first_key;
     return handleEvent(EVENT_IMMEDIATE, 0);
-  } else if (dir_probe(&key, vol, &earliest_dir, &last_collision) ||
-      dir_lookaside_probe(&key, vol, &earliest_dir, NULL))
-  {
+  } else if (dir_probe(&key, vol, &earliest_dir, &last_collision) || dir_lookaside_probe(&key,
vol, &earliest_dir, NULL)) {
     dir = earliest_dir;
     SET_HANDLER(&self::openReadStartEarliest);
     if ((zret = do_read_call(&key)) == EVENT_RETURN) {
@@ -985,7 +945,7 @@ CacheVC::openReadStartEarliest(int /* event ATS_UNUSED */, Event * /*
e ATS_UNUS
     // It's OK if there's a writer for this alternate, we can wait on it.
     if (od && od->has_writer(earliest_key)) {
       wake_up_thread = mutex->thread_holding;
-      od->waiting_for(earliest_key, this, 0);
+      od->wait_for(earliest_key, this, 0);
       lock.release();
       // The SM must be signaled that the cache read is open even if we haven't got the earliest
frag
       // yet because otherwise it won't set up the read side of the tunnel before the write
side finishes

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e92da07f/iocore/cache/CacheWrite.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheWrite.cc b/iocore/cache/CacheWrite.cc
index f6ff4d4..7d6e405 100644
--- a/iocore/cache/CacheWrite.cc
+++ b/iocore/cache/CacheWrite.cc
@@ -1516,11 +1516,11 @@ Lagain:
         Debug("amc", "[CacheVC::openWriteMain] writing empty earliest");
       } else {
         // go on the wait list
-        od->waiting_for(earliest_key, this, 0);
+        od->wait_for(earliest_key, this, 0);
         not_writing = true;
       }
     } else if (od->is_write_active(earliest_key, write_pos)) {
-      od->waiting_for(earliest_key, this, write_pos);
+      od->wait_for(earliest_key, this, write_pos);
       not_writing = true;
     } else if (alternate.is_frag_cached(fragment)) {
       not_writing = true;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e92da07f/iocore/cache/P_CacheDir.h
----------------------------------------------------------------------
diff --git a/iocore/cache/P_CacheDir.h b/iocore/cache/P_CacheDir.h
index 0571150..10487fa 100644
--- a/iocore/cache/P_CacheDir.h
+++ b/iocore/cache/P_CacheDir.h
@@ -300,8 +300,10 @@ struct OpenDirEntry {
   /// Get the fragment key for a specific @a offset.
   CacheKey const &key_for(CacheKey const &alt_key, int64_t offset);
   /** Wait for a fragment to be written.
+
+      @return @c false if there is no writer that is scheduled to write that fragment.
    */
-  self &waiting_for(CacheKey const &alt_key, CacheVC *vc, int64_t offset);
+  bool wait_for(CacheKey const &alt_key, CacheVC *vc, int64_t offset);
   /// Close out anything related to this writer
   self &close_writer(CacheKey const &alt_key, CacheVC *vc);
 };

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e92da07f/iocore/cache/P_CacheHttp.h
----------------------------------------------------------------------
diff --git a/iocore/cache/P_CacheHttp.h b/iocore/cache/P_CacheHttp.h
index 54fb523..458fb9e 100644
--- a/iocore/cache/P_CacheHttp.h
+++ b/iocore/cache/P_CacheHttp.h
@@ -91,6 +91,9 @@ struct CacheHTTPInfoVector {
       } f;
     };
     ///@}
+    /// Check if there are any writers.
+    /// @internal Need to augment this at some point to check for writers to a specific offset.
+    bool has_writers() const;
   };
 
   typedef CacheArray<Item> InfoVector;
@@ -135,7 +138,8 @@ struct CacheHTTPInfoVector {
   /// Indicate if a VC is currently writing to the fragment with this @a offset.
   bool is_write_active(CacheKey const &alt_key, int64_t offset);
   /// Mark a CacheVC as waiting for the fragment containing the byte at @a offset.
-  self &waiting_for(CacheKey const &alt_key, CacheVC *vc, int64_t offset);
+  /// @return @c false if there is no writer scheduled to write that offset.
+  bool wait_for(CacheKey const &alt_key, CacheVC *vc, int64_t offset);
   /// Get the fragment key for a specific @a offset.
   CacheKey const &key_for(CacheKey const &alt_key, int64_t offset);
   /// Close out anything related to this writer
@@ -256,6 +260,12 @@ protected:
   bool _pending_range_shift_p;
 };
 
+TS_INLINE bool
+CacheHTTPInfoVector::Item::has_writers() const
+{
+  return NULL != _writers.head;
+}
+
 TS_INLINE CacheHTTPInfo *
 CacheHTTPInfoVector::get(int idx)
 {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e92da07f/proxy/http/HttpCacheSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpCacheSM.cc b/proxy/http/HttpCacheSM.cc
index c1dc8ba..67a3447 100644
--- a/proxy/http/HttpCacheSM.cc
+++ b/proxy/http/HttpCacheSM.cc
@@ -277,6 +277,11 @@ HttpCacheSM::open_partial_read(HTTPHdr* client_request_hdr)
   // Simple because this requires an active write VC so we know the object is there (no retries).
   ink_assert(NULL != cache_write_vc);
 
+  // If this is a partial fill there will be a cache read VC. Resetting it to be used is
challenging
+  // because it requires digging in to the internals of the VC or expanding its interface.
At present
+  // it's better to just close it and re-open one that we know is valid with regard to the
write VC.
+  this->close_read();
+
   SET_HANDLER(&HttpCacheSM::state_cache_open_partial_read);
   open_read_cb = false;
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e92da07f/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index e2e0b95..9b9238c 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1547,6 +1547,9 @@ HttpSM::handle_api_return()
         t_state.source = HttpTransact::SOURCE_CACHE;
         HTTP_SM_SET_DEFAULT_HANDLER(&HttpSM::state_cache_open_partial_read);
         cache_sm.cache_write_vc = save_write_vc;
+        // Close the read VC if it's there because it's less work than trying to reset the
existing
+        // one (which doesn't have the ODE attached).
+        cache_sm.close_read();
         pending_action = cache_sm.open_partial_read(&t_state.hdr_info.client_request);
         cache_sm.cache_write_vc = NULL;
       } else {


Mime
View raw message