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/09 01:10:54 UTC
trafficserver git commit: Fixed breakage of single range request.
Repository: trafficserver
Updated Branches:
refs/heads/ts-974-5-3-x 2fb305bb4 -> e18407ca3
Fixed breakage of single range request.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/e18407ca
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/e18407ca
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/e18407ca
Branch: refs/heads/ts-974-5-3-x
Commit: e18407ca3a8d2f974fe41b300a803ede96c493bb
Parents: 2fb305b
Author: Alan M. Carroll <so...@yahoo-inc.com>
Authored: Fri May 8 18:10:30 2015 -0500
Committer: Alan M. Carroll <so...@yahoo-inc.com>
Committed: Fri May 8 18:10:30 2015 -0500
----------------------------------------------------------------------
iocore/cache/Cache.cc | 7 ++
iocore/cache/CacheHttp.cc | 2 +-
iocore/cache/CacheRead.cc | 238 ++++++-----------------------------
iocore/cache/I_Cache.h | 11 +-
iocore/cache/P_CacheInternal.h | 17 ++-
iocore/cluster/P_ClusterCache.h | 3 +-
proxy/http/HttpSM.cc | 1 +
7 files changed, 64 insertions(+), 215 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e18407ca/iocore/cache/Cache.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
index c5d8d58..478e732 100644
--- a/iocore/cache/Cache.cc
+++ b/iocore/cache/Cache.cc
@@ -508,6 +508,13 @@ bool CacheVC::set_pin_in_cache(time_t time_pin)
return true;
}
+void
+CacheVC::set_content_range(HTTPRangeSpec const& r)
+{
+ resp_range.getRangeSpec() = r;
+ resp_range.start();
+}
+
bool
CacheVC::get_uncached(HTTPRangeSpec const& req, HTTPRangeSpec& result)
{
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e18407ca/iocore/cache/CacheHttp.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheHttp.cc b/iocore/cache/CacheHttp.cc
index 6078d22..4b66ea6 100644
--- a/iocore/cache/CacheHttp.cc
+++ b/iocore/cache/CacheHttp.cc
@@ -424,7 +424,7 @@ CacheRange::start()
if (_r.hasRanges()) {
_offset = _r[_idx = 0]._min;
- _pending_range_shift_p = true;
+ _pending_range_shift_p = _r.isMulti();
zret = true;
}
return zret;
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e18407ca/iocore/cache/CacheRead.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/CacheRead.cc b/iocore/cache/CacheRead.cc
index 55cd34c..e1a8ee1 100644
--- a/iocore/cache/CacheRead.cc
+++ b/iocore/cache/CacheRead.cc
@@ -227,25 +227,29 @@ CacheVC::get_http_range_boundary_string(int* len) const
int64_t
CacheVC::get_effective_content_size()
{
- return this->is_http_partial_content() ? resp_range.calcContentLength() : alternate.object_size_get();
+ return resp_range.hasRanges() ? resp_range.calcContentLength() : alternate.object_size_get();
}
-HTTPRangeSpec&
-CacheVC::get_http_range_spec()
-{
- return resp_range.getRangeSpec();
-}
-
-bool
-CacheVC::is_http_partial_content()
+int
+CacheVC::closeReadAndFree(int /* event ATS_UNUSED */, Event */* e ATS_UNUSED */)
{
- return resp_range.hasRanges();
+// cancel_trigger(); // ??
+ if (od) {
+ CACHE_TRY_LOCK(lock, vol->mutex, mutex->thread_holding);
+ if (!lock.is_locked()) {
+ SET_HANDLER(&CacheVC::closeReadAndFree);
+ VC_SCHED_LOCK_RETRY();
+ }
+ vol->close_read(this);
+ }
+ return free_CacheVC(this);
}
int
CacheVC::openReadFromWriterFailure(int event, Event *e)
{
- od = NULL;
+ // od = NULL;
+ vol->close_read(this);
vector.clear(false);
CACHE_INCREMENT_DYN_STAT(cache_read_failure_stat);
CACHE_INCREMENT_DYN_STAT(cache_read_busy_failure_stat);
@@ -255,50 +259,6 @@ CacheVC::openReadFromWriterFailure(int event, Event *e)
}
int
-CacheVC::openReadChooseWriter(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */)
-{
-// intptr_t err = ECACHE_DOC_BUSY;
-// CacheVC *w = NULL;
-
- ink_assert(od->mutex->thread_holding == mutex->thread_holding && write_vc == NULL);
-
- if (frag_type != CACHE_FRAG_TYPE_HTTP) {
- ink_release_assert(! "[amc] Fix reader from writer for non-HTTP");
- }
-#ifdef HTTP_CACHE
- else {
- write_vector = &od->vector;
- int write_vec_cnt = write_vector->count();
- for (int c = 0; c < write_vec_cnt; c++)
- vector.insert(write_vector->get(c));
- if (!vector.count()) {
- // [amc] Not sure of the utility of this now
- if (od->reading_vec) {
- // the writer(s) are reading the vector, so there is probably
- // an old vector. Since this reader came before any of the
- // current writers, we should return the old data
- od = NULL;
- return EVENT_RETURN;
- }
- return -ECACHE_NO_DOC;
- }
- if (cache_config_select_alternate) {
- alternate_index = HttpTransactCache::SelectFromAlternates(&vector, &request, params);
- if (alternate_index < 0)
- return -ECACHE_ALT_MISS;
- } else
- alternate_index = 0;
- vector.clear(false);
- DDebug("cache_read_agg",
- "%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_active, write_vc);
- }
-#endif // HTTP_CACHE
- return EVENT_NONE;
-}
-
-int
CacheVC::openReadFromWriter(int event, Event *e)
{
if (!f.read_from_writer_called) {
@@ -314,33 +274,29 @@ CacheVC::openReadFromWriter(int event, Event *e)
f.read_from_writer_called = 1;
}
cancel_trigger();
- intptr_t err = ECACHE_DOC_BUSY;
DDebug("cache_read_agg", "%p: key: %X In openReadFromWriter", this, first_key.slice32(1));
-#ifndef READ_WHILE_WRITER
- return openReadFromWriterFailure(CACHE_EVENT_OPEN_READ_FAILED, (Event *)-err);
-#else
+
if (_action.cancelled) {
- od = NULL; // only open for read so no need to close
- return free_CacheVC(this);
+ return this->closeReadAndFree(0, NULL);
+// od = NULL; // only open for read so no need to close
+// return free_CacheVC(this);
}
CACHE_TRY_LOCK(lock, vol->mutex, mutex->thread_holding);
if (!lock.is_locked())
VC_SCHED_LOCK_RETRY();
- od = vol->open_read(&first_key); // recheck in case the lock failed
- if (!od) {
+ if (!od && NULL == (od = vol->open_read(&first_key))) {
MUTEX_RELEASE(lock);
write_vc = NULL;
SET_HANDLER(&CacheVC::openReadStartHead);
return openReadStartHead(event, e);
- } else
- ink_assert(od == vol->open_read(&first_key));
+ }
CACHE_TRY_LOCK(lock_od, od->mutex, mutex->thread_holding);
if (!lock_od.is_locked())
VC_SCHED_LOCK_RETRY();
if (od->open_writer) {
- // Can't pick a writer yet, so wait for it.
+ // Alternates are in flux, wait for origin server response to update them.
if (!od->open_waiting.in(this)) {
wake_up_thread = mutex->thread_holding;
od->open_waiting.push(this);
@@ -352,6 +308,8 @@ CacheVC::openReadFromWriter(int event, Event *e)
MUTEX_RELEASE(lock); // we have the OD lock now, don't need the vol lock.
if (write_vc && CACHE_ALT_INDEX_DEFAULT != (alternate_index = get_alternate_index(&(od->vector), write_vc->earliest_key))) {
+ // Found the alternate for our write VC. Really, though, if we have a write_vc we should never fail to get
+ // the alternate - we should probably check for that.
alternate.copy_shallow(od->vector.get(alternate_index));
key = earliest_key = alternate.object_key_get();
doc_len = alternate.object_size_get();
@@ -359,136 +317,22 @@ CacheVC::openReadFromWriter(int event, Event *e)
SET_HANDLER(&CacheVC::openReadStartEarliest);
return openReadStartEarliest(event, e);
} else {
- int ret = openReadChooseWriter(event, e);
- if (ret < 0) {
- MUTEX_RELEASE(lock_od);
- SET_HANDLER(&CacheVC::openReadFromWriterFailure);
- return openReadFromWriterFailure(CACHE_EVENT_OPEN_READ_FAILED, reinterpret_cast<Event *>(ret));
- } else if (ret == EVENT_RETURN) {
- MUTEX_RELEASE(lock_od);
- SET_HANDLER(&CacheVC::openReadStartHead);
- return openReadStartHead(event, e);
- } else if (ret == EVENT_CONT) {
- ink_assert(!write_vc);
- VC_SCHED_WRITER_RETRY();
- } else
- ink_assert(write_vc);
- }
-#ifdef HTTP_CACHE
- OpenDirEntry *cod = od;
-#endif
- od = NULL;
- // someone is currently writing the document
- if (write_vc->closed < 0) {
- MUTEX_RELEASE(lock_od);
- write_vc = NULL;
- // writer aborted, continue as if there is no writer
- SET_HANDLER(&CacheVC::openReadStartHead);
- return openReadStartHead(EVENT_IMMEDIATE, 0);
- }
- // allow reading from unclosed writer for http requests only.
- ink_assert(frag_type == CACHE_FRAG_TYPE_HTTP || write_vc->closed);
- if (!write_vc->closed && !write_vc->fragment) {
- if (!cache_config_read_while_writer || frag_type != CACHE_FRAG_TYPE_HTTP) {
- MUTEX_RELEASE(lock_od);
- return openReadFromWriterFailure(CACHE_EVENT_OPEN_READ_FAILED, (Event *) - err);
- }
- DDebug("cache_read_agg", "%p: key: %X writer: closed:%d, fragment:%d, retry: %d", this, first_key.slice32(1), write_vc->closed,
- write_vc->fragment, writer_lock_retry);
- VC_SCHED_WRITER_RETRY();
- }
-
- CACHE_TRY_LOCK(writer_lock, write_vc->mutex, mutex->thread_holding);
- if (!writer_lock.is_locked()) {
- DDebug("cache_read_agg", "%p: key: %X lock miss", this, first_key.slice32(1));
- VC_SCHED_LOCK_RETRY();
- }
- MUTEX_RELEASE(lock);
-
- if (!write_vc->io.ok())
- return openReadFromWriterFailure(CACHE_EVENT_OPEN_READ_FAILED, (Event *)-err);
-#ifdef HTTP_CACHE
- if (frag_type == CACHE_FRAG_TYPE_HTTP) {
- DDebug("cache_read_agg", "%p: key: %X http passed stage 1, closed: %d, frag: %d", this, first_key.slice32(1), write_vc->closed,
- write_vc->fragment);
- if (!write_vc->alternate.valid())
- return openReadFromWriterFailure(CACHE_EVENT_OPEN_READ_FAILED, (Event *)-err);
- alternate.copy(&write_vc->alternate);
- vector.insert(&alternate);
- alternate.object_key_get(&key);
- write_vc->f.readers = 1;
- if (!(write_vc->f.update && write_vc->total_len == 0)) {
- key = write_vc->earliest_key;
- if (!write_vc->closed)
- alternate.object_size_set(write_vc->vio.nbytes);
- else
- alternate.object_size_set(write_vc->total_len);
- } else {
- key = write_vc->update_key;
- ink_assert(write_vc->closed);
- DDebug("cache_read_agg", "%p: key: %X writer header update", this, first_key.slice32(1));
- // Update case (b) : grab doc_len from the writer's alternate
- doc_len = alternate.object_size_get();
- if (write_vc->update_key == cod->single_doc_key && (cod->move_resident_alt || write_vc->f.rewrite_resident_alt) &&
- write_vc->first_buf._ptr()) {
- // the resident alternate is being updated and its a
- // header only update. The first_buf of the writer has the
- // document body.
- Doc *doc = (Doc *)write_vc->first_buf->data();
- writer_buf = new_IOBufferBlock(write_vc->first_buf, doc->data_len(), doc->prefix_len());
- MUTEX_RELEASE(writer_lock);
- ink_assert(doc_len == doc->data_len());
- length = doc_len;
- f.single_fragment = 1;
- doc_pos = 0;
- earliest_key = key;
- dir_clean(&first_dir);
- dir_clean(&earliest_dir);
- SET_HANDLER(&CacheVC::openReadFromWriterMain);
- CACHE_INCREMENT_DYN_STAT(cache_read_busy_success_stat);
- return callcont(CACHE_EVENT_OPEN_READ);
+ if (cache_config_select_alternate) {
+ alternate_index = HttpTransactCache::SelectFromAlternates(&vector, &request, params);
+ if (alternate_index < 0) {
+ MUTEX_RELEASE(lock_od);
+ SET_HANDLER(&CacheVC::openReadFromWriterFailure);
+ return openReadFromWriterFailure(CACHE_EVENT_OPEN_READ_FAILED, reinterpret_cast<Event *>(-ECACHE_ALT_MISS));
}
- // want to snarf the new headers from the writer
- // and then continue as if nothing happened
- last_collision = NULL;
- MUTEX_RELEASE(writer_lock);
- SET_HANDLER(&CacheVC::openReadStartEarliest);
- return openReadStartEarliest(event, e);
+ } else {
+ alternate_index = 0;
}
- } else {
-#endif // HTTP_CACHE
- DDebug("cache_read_agg", "%p: key: %X non-http passed stage 1", this, first_key.slice32(1));
- key = write_vc->earliest_key;
-#ifdef HTTP_CACHE
- }
-#endif
- if (write_vc->fragment) {
- doc_len = write_vc->vio.nbytes;
- last_collision = NULL;
- DDebug("cache_read_agg", "%p: key: %X closed: %d, fragment: %d, len: %d starting first fragment", this, first_key.slice32(1),
- write_vc->closed, write_vc->fragment, (int)doc_len);
- MUTEX_RELEASE(writer_lock);
- // either a header + body update or a new document
- SET_HANDLER(&CacheVC::openReadStartEarliest);
- return openReadStartEarliest(event, e);
+ MUTEX_RELEASE(lock_od);
+ SET_HANDLER(&CacheVC::openReadStartHead);
+ return openReadStartHead(event, e);
}
- writer_buf = write_vc->blocks;
- writer_offset = write_vc->offset;
- length = write_vc->length;
- // copy the vector
- f.single_fragment = !write_vc->fragment; // single fragment doc
- doc_pos = 0;
- earliest_key = write_vc->earliest_key;
- ink_assert(earliest_key == key);
- doc_len = write_vc->total_len;
- dir_clean(&first_dir);
- dir_clean(&earliest_dir);
- DDebug("cache_read_agg", "%p: key: %X %X: single fragment read", this, first_key.slice32(1), key.slice32(0));
- MUTEX_RELEASE(writer_lock);
- SET_HANDLER(&CacheVC::openReadFromWriterMain);
- CACHE_INCREMENT_DYN_STAT(cache_read_busy_success_stat);
- return callcont(CACHE_EVENT_OPEN_READ);
-#endif // READ_WHILE_WRITER
+ ink_assert(false);
+ return EVENT_DONE; // should not get here.
}
int
@@ -842,7 +686,7 @@ CacheVC::openReadWaitEarliest(int evid, Event*)
// there's no explicit earliest frag.
lock.release();
SET_HANDLER(&self::openReadStartHead);
- od = NULL;
+// 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)) {
@@ -1208,11 +1052,6 @@ CacheVC::openReadStartHead(int event, Event *e)
alternate.object_key_get(&key);
doc_len = alternate.object_size_get();
- // Handle any range related setup.
- if (!resp_range.init(&request)) { // shouldn't this have been checked earlier?
- err = ECACHE_INVALID_RANGE;
- goto Ldone;
- }
// If the object length is known we can check the range.
// Otherwise we have to leave it vague and talk to the origin to get full length info.
if (alternate.m_alt->m_flag.content_length_p && !resp_range.apply(doc_len)) {
@@ -1221,7 +1060,6 @@ CacheVC::openReadStartHead(int event, Event *e)
}
if (resp_range.isMulti())
resp_range.setContentTypeFromResponse(alternate.response_get()).generateBoundaryStr(earliest_key);
-// alternate.get_uncached(resp_range.getRangeSpec(), uncached_range.getRangeSpec());
if (key == doc->key) { // is this my data?
f.single_fragment = doc->single_fragment();
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e18407ca/iocore/cache/I_Cache.h
----------------------------------------------------------------------
diff --git a/iocore/cache/I_Cache.h b/iocore/cache/I_Cache.h
index c75430d..187343b 100644
--- a/iocore/cache/I_Cache.h
+++ b/iocore/cache/I_Cache.h
@@ -245,16 +245,9 @@ struct CacheVConnection : public VConnection {
*/
virtual void set_full_content_length(int64_t) = 0;
- /** Get the range spec for the response (request ranges modifed by content length).
- @internal Need better comment - this is the range spec used for a response from ATS to the user agent
- from cached data. Even better we have potentially 2 response ranges - that from the origin server to
- ATS and that from ATS to the user agent which are only somewhat similar, depending on what exactly
- is in the cache at the moment.
+ /** Set the output ranges for the content.
*/
- virtual HTTPRangeSpec& get_http_range_spec() = 0;
-
- /// Check if this is HTTP partial content (range request/response).
- virtual bool is_http_partial_content() = 0;
+ virtual void set_content_range(HTTPRangeSpec const& range) = 0;
/// Get the unchanged ranges for the request range @a req.
/// If @a req is empty it is treated as a full request (non-partial).
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e18407ca/iocore/cache/P_CacheInternal.h
----------------------------------------------------------------------
diff --git a/iocore/cache/P_CacheInternal.h b/iocore/cache/P_CacheInternal.h
index d3678d2..0aca4f2 100644
--- a/iocore/cache/P_CacheInternal.h
+++ b/iocore/cache/P_CacheInternal.h
@@ -317,7 +317,7 @@ struct CacheVC: public CacheVConnection
int openReadFromWriter(int event, Event *e);
int openReadFromWriterMain(int event, Event *e);
int openReadFromWriterFailure(int event, Event *);
- int openReadChooseWriter(int event, Event *e);
+ // int openReadChooseWriter(int event, Event *e);
int openWriteCloseDir(int event, Event *e);
int openWriteCloseHeadDone(int event, Event *e);
@@ -338,6 +338,8 @@ struct CacheVC: public CacheVConnection
int updateVecWrite(int event, Event *e);
int updateWriteStateFromRange();
+ int closeReadAndFree(int event, Event *e);
+
int removeEvent(int event, Event *e);
int linkWrite(int event, Event *e);
@@ -390,10 +392,18 @@ struct CacheVC: public CacheVConnection
virtual char const* get_http_range_boundary_string(int* len) const;
virtual int64_t get_effective_content_size();
virtual void set_full_content_length(int64_t size);
- virtual HTTPRangeSpec& get_http_range_spec();
- virtual bool is_http_partial_content();
virtual bool get_uncached(HTTPRangeSpec const& req, HTTPRangeSpec& result);
+ /** This sets a range for data flowing in to the cache VC.
+ The CacheVC will write the incoming data to this part of the overall object.
+ @internal It's done this way to isolate the CacheVC from parsing range separators
+ in multi-range responses.
+ */
virtual int64_t set_inbound_range(int64_t min, int64_t max);
+ /** Select the ranges to apply to the content.
+ @internal In this case the CacheVC has to know the entire set of ranges so it can correctly
+ compute the actual output size (vs. the content size).
+ */
+ virtual void set_content_range(HTTPRangeSpec const& range);
#endif
@@ -883,6 +893,7 @@ Vol::open_write(CacheVC *cont)
#endif
ink_assert(NULL == cont->od);
if (NULL != (cont->od = open_dir.open_entry(this, cont->first_key, true))) {
+ cont->write_vector = &cont->od->vector;
#ifdef CACHE_STAT_PAGES
ink_assert(cont->mutex->thread_holding == this_ethread());
ink_assert(!cont->stat_link.next && !cont->stat_link.prev);
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e18407ca/iocore/cluster/P_ClusterCache.h
----------------------------------------------------------------------
diff --git a/iocore/cluster/P_ClusterCache.h b/iocore/cluster/P_ClusterCache.h
index 9f85ec5..000f764 100644
--- a/iocore/cluster/P_ClusterCache.h
+++ b/iocore/cluster/P_ClusterCache.h
@@ -361,11 +361,10 @@ struct ClusterVConnectionBase : public CacheVConnection {
// I think the best approach is to foist the work off to the source peer and have it do
// the range formatting which we then just pass through. For now, this just prevents
// link problems so I can get the base case to work.
+ virtual void set_content_range(HTTPRangeSpec const&) { return; }
virtual char const* get_http_range_boundary_string(int*) const { return NULL; }
virtual int64_t get_effective_content_size() { return this->get_object_size(); }
virtual void set_full_content_length(int64_t) { } // only used when writing to cache
- virtual HTTPRangeSpec& get_http_range_spec() { return resp_range.getRangeSpec(); }
- virtual bool is_http_partial_content() { return false; }
// Set the timeouts associated with this connection.
// active_timeout is for the total elasped time of the connection.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e18407ca/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 9b9238c..f22bfa1 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -2547,6 +2547,7 @@ HttpSM::state_cache_open_partial_read(int event, void *data)
cache_sm.cache_read_vc->get_http_info(&t_state.cache_info.object_read);
ink_assert(t_state.cache_info.object_read != 0);
+ cache_sm.cache_read_vc->set_content_range(t_state.hdr_info.request_range);
t_state.next_action = HttpTransact::SM_ACTION_SERVE_FROM_CACHE;
t_state.api_next_action = HttpTransact::SM_ACTION_API_SEND_RESPONSE_HDR;