trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a..@apache.org
Subject [2/2] trafficserver git commit: Fixed problem with ODE not being cleared on cache read VCs.
Date Mon, 04 May 2015 03:27:09 GMT
Fixed problem with ODE not being cleared on cache read VCs.


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

Branch: refs/heads/ts-974-5-3-x
Commit: 2fb305bb48355cace90c984faa0505fa2b93bc9d
Parents: e92da07
Author: Alan M. Carroll <solidwallofcode@yahoo-inc.com>
Authored: Sun May 3 22:26:17 2015 -0500
Committer: Alan M. Carroll <solidwallofcode@yahoo-inc.com>
Committed: Sun May 3 22:26:17 2015 -0500

----------------------------------------------------------------------
 iocore/cache/Cache.cc          |  3 +-
 iocore/cache/CacheDir.cc       | 76 +++++++++++++++----------------------
 iocore/cache/CacheRead.cc      |  7 ++--
 iocore/cache/CacheVol.cc       |  5 ++-
 iocore/cache/CacheWrite.cc     | 20 ++++++----
 iocore/cache/P_CacheDir.h      | 18 ++++-----
 iocore/cache/P_CacheInternal.h | 22 +++++------
 iocore/cache/P_CacheVol.h      | 10 ++---
 8 files changed, 74 insertions(+), 87 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2fb305bb/iocore/cache/Cache.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
index c685c11..c5d8d58 100644
--- a/iocore/cache/Cache.cc
+++ b/iocore/cache/Cache.cc
@@ -582,6 +582,7 @@ Vol::close_read(CacheVC *cont)
   EThread *t = cont->mutex->thread_holding;
   ink_assert(t == this_ethread());
   ink_assert(t == mutex->thread_holding);
+  open_dir.close_entry(cont);
   if (dir_is_empty(&cont->earliest_dir))
     return 1;
   int i = dir_evac_bucket(&cont->earliest_dir);
@@ -2883,7 +2884,7 @@ CacheVC::removeEvent(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED
*/)
       goto Lfree;
     }
     if (!f.remove_aborted_writers) {
-      if (vol->open_write(this, true, 1)) {
+      if (vol->open_write(this)) {
         // writer  exists
         ink_release_assert(od = vol->open_read(&key));
         od->dont_update_directory = 1;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2fb305bb/iocore/cache/CacheDir.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheDir.cc b/iocore/cache/CacheDir.cc
index 038e6f0..6eedc46 100644
--- a/iocore/cache/CacheDir.cc
+++ b/iocore/cache/CacheDir.cc
@@ -61,51 +61,39 @@ OpenDir::OpenDir()
   SET_HANDLER(&OpenDir::signal_readers);
 }
 
-/*
-   If allow_if_writers is false, open_write fails if there are other writers.
-   max_writers sets the maximum number of concurrent writers that are
-   allowed. Only The first writer can set the max_writers. It is ignored
-   for later writers.
-   Returns 1 on success and 0 on failure.
-   */
-int
-OpenDir::open_write(CacheVC *cont, int /* allow_if_writers */, int /* max_writers */)
+OpenDirEntry*
+OpenDir::open_entry(Vol* vol, CryptoHash const& key, bool force_p)
 {
-  ink_assert(cont->vol->mutex->thread_holding == this_ethread());
-  unsigned int h = cont->first_key.slice32(0);
+  ink_assert(vol->mutex->thread_holding == this_ethread());
+  unsigned int h = key.slice32(0);
   int b = h % OPEN_DIR_BUCKETS;
   for (OpenDirEntry *d = bucket[b].head; d; d = d->link.next) {
-    if (!(d->first_key == cont->first_key))
+    if (!(d->first_key == key))
       continue;
-//    if (allow_if_writers && d->num_writers < d->max_writers) {
-//      d->writers.push(cont);
-    // [amc] Need to think if we want to track writers per object and not just per alt.
-    // useful to know when to close out the OpenDirEntry.
-      d->num_writers++;
-      cont->od = d;
-      cont->write_vector = &d->vector;
-      return 1;
-//    }
-    return 0;
+    ++(d->num_active);
+//    cont->od = d;
+//    cont->write_vector = &d->vector;
+    return d;
   }
+
+  if (!force_p)
+    return NULL;
+
   OpenDirEntry *od = THREAD_ALLOC(openDirEntryAllocator,
-                                  cont->mutex->thread_holding);
-//  od->readers.head = NULL;
-//  od->writers.push(cont);
+                                  vol->mutex->thread_holding);
   od->mutex = new_ProxyMutex();
-  od->first_key = cont->first_key;
-  od->num_writers = 1;
-//  od->max_writers = max_writers;
+  od->first_key = key;
+  od->num_active = 1;
   od->vector.data.data = &od->vector.data.fast_data[0];
   od->dont_update_directory = 0;
   od->move_resident_alt = 0;
   od->reading_vec = 0;
   od->writing_vec = 0;
   dir_clear(&od->first_dir);
-  cont->od = od;
-  cont->write_vector = &od->vector;
+//  cont->od = od;
+//  cont->write_vector = &od->vector;
   bucket[b].push(od);
-  return 1;
+  return od;
 }
 
 int
@@ -133,25 +121,22 @@ OpenDir::signal_readers(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED
*/)
   return 0;
 }
 
-int
-OpenDir::close_write(CacheVC *cont)
+void
+OpenDir::close_entry(CacheVC* vc)
 {
-  ink_assert(cont->vol->mutex->thread_holding == this_ethread());
-//  cont->od->writers.remove(cont);
-  if (--(cont->od->num_writers) < 1) {
-    unsigned int h = cont->first_key.slice32(0);
+  ink_assert(vc->vol->mutex->thread_holding == this_ethread());
+  if (--(vc->od->num_active) < 1) {
+    unsigned int h = vc->od->first_key.slice32(0);
     int b = h % OPEN_DIR_BUCKETS;
-    bucket[b].remove(cont->od);
-//    delayed_readers.append(cont->od->readers);
-//    signal_readers(0, 0);
-    cont->od->vector.clear();
-    cont->od->mutex = 0;
-    THREAD_FREE(cont->od, openDirEntryAllocator, cont->mutex->thread_holding);
+    bucket[b].remove(vc->od);
+    vc->od->vector.clear();
+    vc->od->mutex = 0;
+    THREAD_FREE(vc->od, openDirEntryAllocator, vc->vol->mutex->thread_holding);
   }
-  cont->od = NULL;
-  return 0;
+  vc->od = NULL;
 }
 
+# if 0
 OpenDirEntry *
 OpenDir::open_read(INK_MD5 *key)
 {
@@ -163,7 +148,6 @@ OpenDir::open_read(INK_MD5 *key)
   return NULL;
 }
 
-# if 0
 int
 OpenDirEntry::wait(CacheVC *cont, int msec)
 {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2fb305bb/iocore/cache/CacheRead.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheRead.cc b/iocore/cache/CacheRead.cc
index 726c834..55cd34c 100644
--- a/iocore/cache/CacheRead.cc
+++ b/iocore/cache/CacheRead.cc
@@ -290,9 +290,9 @@ CacheVC::openReadChooseWriter(int /* event ATS_UNUSED */, Event * /* e
ATS_UNUSE
       alternate_index = 0;
     vector.clear(false);
     DDebug("cache_read_agg",
-          "%p: key: %X eKey: %d # alts: %d, ndx: %d, # writers: %d writer: %p",
+          "%p: key: %X eKey: %d # alts: %d, ndx: %d, # active: %d writer: %p",
           this, first_key.slice32(1), write_vc->earliest_key.slice32(1),
-          vector.count(), alternate_index, od->num_writers, write_vc);
+          vector.count(), alternate_index, od->num_active, write_vc);
   }
 #endif // HTTP_CACHE
   return EVENT_NONE;
@@ -956,7 +956,8 @@ CacheVC::openReadStartEarliest(int /* event ATS_UNUSED */, Event * /*
e ATS_UNUS
       return callcont(CACHE_EVENT_OPEN_READ); // must signal read is open
     } else if (frag_type == CACHE_FRAG_TYPE_HTTP) {
       // don't want any writers while we are evacuating the vector
-      if (!vol->open_write(this, false, 1)) {
+      ink_release_assert(!"[amc] Not handling multiple writers with vector evacuate");
+      if (!vol->open_write(this)) {
         Doc *doc1 = (Doc *)first_buf->data();
         uint32_t len = this->load_http_info(write_vector, doc1);
         ink_assert(len == doc1->hlen && write_vector->count() > 0);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2fb305bb/iocore/cache/CacheVol.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheVol.cc b/iocore/cache/CacheVol.cc
index 47f5775..857027e 100644
--- a/iocore/cache/CacheVol.cc
+++ b/iocore/cache/CacheVol.cc
@@ -413,8 +413,9 @@ CacheVC::scanOpenWrite(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED
*/)
     }
 
     Debug("cache_scan", "trying for writer lock");
-    if (vol->open_write(this, false, 1)) {
-      writer_lock_retry++;
+    if (vol->open_write(this)) {
+      // [amc] This tried to restrict to one writer, must fix at some point.
+      ++writer_lock_retry;
       SET_HANDLER(&CacheVC::scanOpenWrite);
       mutex->thread_holding->schedule_in_local(this, scan_msec_delay);
       return EVENT_CONT;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2fb305bb/iocore/cache/CacheWrite.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheWrite.cc b/iocore/cache/CacheWrite.cc
index 7d6e405..20d6bdb 100644
--- a/iocore/cache/CacheWrite.cc
+++ b/iocore/cache/CacheWrite.cc
@@ -107,7 +107,7 @@ CacheVC::updateVector(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED
*/)
       alternate_index = write_vector->insert(&alternate, alternate_index);
     }
 
-    if (od->move_resident_alt && first_buf._ptr() && !od->has_multiple_writers())
{
+    if (od->move_resident_alt && first_buf._ptr() /* && !od->has_multiple_writers()
*/) {
       Doc *doc = (Doc *)first_buf->data();
       int small_doc = (int64_t)doc->data_len() < (int64_t)cache_config_alt_rewrite_max_size;
       int have_res_alt = doc->key == od->single_doc_key;
@@ -1606,7 +1606,7 @@ CacheVC::openWriteStartDone(int event, Event *e)
     if (!lock.is_locked())
       VC_LOCK_RETRY_EVENT();
 
-    if (_action.cancelled && (!od || !od->has_multiple_writers()))
+    if (_action.cancelled && (!od /* || !od->has_multiple_writers() */ ))
       goto Lcancel;
 
     if (event == AIO_EVENT_DONE) { // vector read done
@@ -1671,15 +1671,17 @@ CacheVC::openWriteStartDone(int event, Event *e)
     }
 
   Lcollision:
-    int if_writers = ((uintptr_t)info == CACHE_ALLOW_MULTIPLE_WRITES);
+//    int if_writers = ((uintptr_t)info == CACHE_ALLOW_MULTIPLE_WRITES);
     if (!od) {
-      if ((err = vol->open_write(this, if_writers, cache_config_http_max_alts > 1 ?
cache_config_http_max_alts : 0)) > 0)
+      if ((err = vol->open_write(this)) > 0)
         goto Lfailure;
+/*
       if (od->has_multiple_writers()) {
         MUTEX_RELEASE(lock);
         SET_HANDLER(&CacheVC::openWriteInit);
         return this->openWriteInit(EVENT_IMMEDIATE, 0);
       }
+*/
     }
     // check for collision
     if (dir_probe(&first_key, vol, &dir, &last_collision)) {
@@ -1724,7 +1726,7 @@ CacheVC::openWriteStartBegin(int /* event ATS_UNUSED */, Event * /*
e ATS_UNUSED
   cancel_trigger();
   if (_action.cancelled)
     return free_CacheVC(this);
-  if (((err = vol->open_write_lock(this, false, 1)) > 0)) {
+  if (((err = vol->open_write_lock(this)) > 0)) {
     CACHE_INCREMENT_DYN_STAT(base_stat + CACHE_STAT_FAILURE);
     free_CacheVC(this);
     _action.continuation->handleEvent(CACHE_EVENT_OPEN_WRITE_FAILED, (void *)-err);
@@ -1784,7 +1786,7 @@ Cache::open_write(Continuation *cont, CacheKey *key, CacheFragType frag_type,
in
   c->f.sync = (options & CACHE_WRITE_OPT_SYNC) == CACHE_WRITE_OPT_SYNC;
   c->pin_in_cache = (uint32_t)apin_in_cache;
 
-  if ((res = c->vol->open_write_lock(c, false, 1)) > 0) {
+  if ((res = c->vol->open_write_lock(c)) > 0) {
     // document currently being written, abort
     CACHE_INCREMENT_DYN_STAT(c->base_stat + CACHE_STAT_FAILURE);
     cont->handleEvent(CACHE_EVENT_OPEN_WRITE_FAILED, (void *)-res);
@@ -1838,7 +1840,7 @@ Cache::open_write(Continuation *cont, CacheKey *key, CacheHTTPInfo *info,
time_t
 
   ink_assert(caches[type] == this);
   intptr_t err = 0;
-  int if_writers = (uintptr_t)info == CACHE_ALLOW_MULTIPLE_WRITES;
+//  int if_writers = (uintptr_t)info == CACHE_ALLOW_MULTIPLE_WRITES;
   CacheVC *c = new_CacheVC(cont);
   ProxyMutex *mutex = cont->mutex;
   c->vio.op = VIO::WRITE;
@@ -1902,13 +1904,15 @@ Cache::open_write(Continuation *cont, CacheKey *key, CacheHTTPInfo
*info, time_t
   {
     CACHE_TRY_LOCK(lock, c->vol->mutex, cont->mutex->thread_holding);
     if (lock.is_locked()) {
-      if ((err = c->vol->open_write(c, if_writers, cache_config_http_max_alts >
1 ? cache_config_http_max_alts : 0)) > 0)
+      if ((err = c->vol->open_write(c)) > 0)
         goto Lfailure;
       // If there are multiple writers, then this one cannot be an update.
       // Only the first writer can do an update. If that's the case, we can
       // return success to the state machine now.;
+/*
       if (c->od->has_multiple_writers())
         goto Lmiss;
+*/
       if (!dir_probe(key, c->vol, &c->dir, &c->last_collision)) {
         if (c->f.update) {
           // fail update because vector has been GC'd

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2fb305bb/iocore/cache/P_CacheDir.h
----------------------------------------------------------------------
diff --git a/iocore/cache/P_CacheDir.h b/iocore/cache/P_CacheDir.h
index 10487fa..1020764 100644
--- a/iocore/cache/P_CacheDir.h
+++ b/iocore/cache/P_CacheDir.h
@@ -254,7 +254,7 @@ struct OpenDirEntry {
   Dir single_doc_dir;         // Directory for the resident alternate
   Dir first_dir;              // Dir for the vector. If empty, a new dir is
                               // inserted, otherwise this dir is overwritten
-  uint16_t num_writers;       // num of current writers
+  uint16_t num_active;        // num of VCs working with this entry
   uint16_t max_writers;       // max number of simultaneous writers allowed
   bool dont_update_directory; // if set, the first_dir is not updated.
   bool move_resident_alt;     // if set, single_doc_dir is inserted.
@@ -280,12 +280,6 @@ struct OpenDirEntry {
 
   //  int wait(CacheVC *c, int msec);
 
-  bool
-  has_multiple_writers()
-  {
-    return num_writers > 1;
-  }
-
   /// Get the alternate index for the @a key.
   int index_of(CacheKey const &key);
   /// Check if there are any writers for the alternate of @a alt_key.
@@ -314,9 +308,13 @@ struct OpenDir : public Continuation {
 
   DLL<OpenDirEntry> bucket[OPEN_DIR_BUCKETS];
 
-  int open_write(CacheVC *c, int allow_if_writers, int max_writers);
-  int close_write(CacheVC *c);
-  OpenDirEntry *open_read(CryptoHash *key);
+  /** Open a live directory entry for @a vc.
+
+      @a force_p is set to @c true to force the entry if it's not already there.
+  */
+  OpenDirEntry* open_entry(Vol* vol, CryptoHash const& key, bool force_p = false);
+  void close_entry(CacheVC *c);
+  //  OpenDirEntry *open_read(CryptoHash *key);
   int signal_readers(int event, Event *e);
 
   OpenDir();

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2fb305bb/iocore/cache/P_CacheInternal.h
----------------------------------------------------------------------
diff --git a/iocore/cache/P_CacheInternal.h b/iocore/cache/P_CacheInternal.h
index 5290248..d3678d2 100644
--- a/iocore/cache/P_CacheInternal.h
+++ b/iocore/cache/P_CacheInternal.h
@@ -848,7 +848,7 @@ CacheVC::writer_done()
 }
 # endif
 
-TS_INLINE int
+TS_INLINE void
 Vol::close_write(CacheVC *cont)
 {
 #ifdef CACHE_STAT_PAGES
@@ -856,12 +856,12 @@ Vol::close_write(CacheVC *cont)
   stat_cache_vcs.remove(cont, cont->stat_link);
   ink_assert(!cont->stat_link.next && !cont->stat_link.prev);
 #endif
-  return open_dir.close_write(cont);
+  open_dir.close_entry(cont);
 }
 
 // Returns 0 on success or a positive error code on failure
 TS_INLINE int
-Vol::open_write(CacheVC *cont, int allow_if_writers, int max_writers)
+Vol::open_write(CacheVC *cont)
 {
   Vol *vol = this;
   bool agg_error = false;
@@ -881,7 +881,8 @@ Vol::open_write(CacheVC *cont, int allow_if_writers, int max_writers)
     m_result->notMigrate = true;
   }
 #endif
-  if (open_dir.open_write(cont, allow_if_writers, max_writers)) {
+  ink_assert(NULL == cont->od);
+  if (NULL != (cont->od = open_dir.open_entry(this, cont->first_key, true))) {
 #ifdef CACHE_STAT_PAGES
     ink_assert(cont->mutex->thread_holding == this_ethread());
     ink_assert(!cont->stat_link.next && !cont->stat_link.prev);
@@ -899,26 +900,23 @@ Vol::close_write_lock(CacheVC *cont)
   CACHE_TRY_LOCK(lock, mutex, t);
   if (!lock.is_locked())
     return -1;
-  return close_write(cont);
+  this->close_write(cont);
+  return 0;
 }
 
 TS_INLINE int
-Vol::open_write_lock(CacheVC *cont, int allow_if_writers, int max_writers)
+Vol::open_write_lock(CacheVC *cont)
 {
   EThread *t = cont->mutex->thread_holding;
   CACHE_TRY_LOCK(lock, mutex, t);
-  if (!lock.is_locked())
-    return -1;
-  return open_write(cont, allow_if_writers, max_writers);
+  return lock.is_locked() ? this->open_write(cont) : -1;
 }
 
 TS_INLINE OpenDirEntry *
 Vol::open_read_lock(INK_MD5 *key, EThread *t)
 {
   CACHE_TRY_LOCK(lock, mutex, t);
-  if (!lock.is_locked())
-    return NULL;
-  return open_dir.open_read(key);
+  return lock.is_locked() ? open_dir.open_entry(this, *key, false) : NULL;
 }
 
 TS_INLINE int

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2fb305bb/iocore/cache/P_CacheVol.h
----------------------------------------------------------------------
diff --git a/iocore/cache/P_CacheVol.h b/iocore/cache/P_CacheVol.h
index 5545992..f1e014f 100644
--- a/iocore/cache/P_CacheVol.h
+++ b/iocore/cache/P_CacheVol.h
@@ -536,10 +536,10 @@ struct Vol : public Continuation {
 
   int recover_data();
 
-  int open_write(CacheVC *cont, int allow_if_writers, int max_writers);
-  int open_write_lock(CacheVC *cont, int allow_if_writers, int max_writers);
-  int close_write(CacheVC *cont);
-  int close_write_lock(CacheVC *cont);
+  int open_write(CacheVC *cont);
+  int open_write_lock(CacheVC *cont);
+  void close_write(CacheVC *cont);
+  int close_write_lock(CacheVC *cont); // can fail lock
   int begin_read(CacheVC *cont);
   int begin_read_lock(CacheVC *cont);
   // unused read-write interlock code
@@ -867,7 +867,7 @@ free_EvacuationBlock(EvacuationBlock *b, EThread *t)
 TS_INLINE OpenDirEntry *
 Vol::open_read(CryptoHash *key)
 {
-  return open_dir.open_read(key);
+  return open_dir.open_entry(this, *key, false);
 }
 
 TS_INLINE int


Mime
View raw message