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/04 05:27:09 UTC
[2/2] trafficserver git commit: Fixed problem with ODE not being
cleared on cache read VCs.
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 <so...@yahoo-inc.com>
Authored: Sun May 3 22:26:17 2015 -0500
Committer: Alan M. Carroll <so...@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