Skip to content

Collapsed forwarding support in ATS core#5377

Closed
ezelkow1 wants to merge 3 commits intoapache:masterfrom
ezelkow1:cf_core
Closed

Collapsed forwarding support in ATS core#5377
ezelkow1 wants to merge 3 commits intoapache:masterfrom
ezelkow1:cf_core

Conversation

@ezelkow1
Copy link
Copy Markdown
Member

This adds a new write fail action, CACHE_WL_FAIL_ACTION_COLLAPSED_FORWARDING. If it fails to obtain a write lock then it will loop for open_write_retries*open_read_retry_time, each time doing a DecideCacheLookup. In the end if it fails to ever either obtain a
write lock or find the object to read then it does the default write fail action of going to the origin.

In testing using duplicated requests I can see multiple simultaneous requests end up in this read loop and eventually get a hit in cache while the original request gets written. Occaisonally there is still an extra hit or two to the origin which I believe is
state machine dependant, so sort of unavoidable, sometimes you can have the original request finish the write while a duplicated request ends up obtaining the write lock directly afterwards and so it hits the origin as well. This may be unavoidable unless cache
lookups are done everywhere before a write.

On an origin fail it will do the default of sending back an ERR_CONNECT_FAIL

@ezelkow1 ezelkow1 added the Core label Apr 25, 2019
@ezelkow1 ezelkow1 added this to the 9.0.0 milestone Apr 25, 2019
@ezelkow1 ezelkow1 self-assigned this Apr 25, 2019
@zwoop zwoop modified the milestones: 9.0.0, 10.0.0 Aug 21, 2019
Comment thread proxy/http/HttpCacheSM.cc Outdated
&cache_key, lookup_url, read_request_hdr, master_sm->t_state.cache_info.object_read,
(time_t)((master_sm->t_state.cache_control.pin_in_cache_for < 0) ? 0 : master_sm->t_state.cache_control.pin_in_cache_for),
retry_write, false);
if (master_sm->t_state.txn_conf->cache_open_write_fail_action != HttpTransact::CACHE_WL_FAIL_ACTION_COLLAPSED_FORWARDING) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case CACHE_EVENT_OPEN_WRITE_FAILED:
// if (master_sm->t_state.txn_conf->cache_open_write_fail_action == //CACHE_OPEN_WRITE_FAIL_ACTION::READ_RETRY) {
// just fall back to open_read_retry settings
//open_write_tries = master_sm->t_state.txn_conf->max_cache_open_write_retries;
//}

if (open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries) {
  // Retry open write;
  open_write_cb = false;
  do_schedule_in();
} else {

Comment thread proxy/http/HttpCacheSM.cc Outdated
Debug("http_cache", "[%" PRId64 "] [state_cache_open_write] cache open write failure INTERVAL. Saw CF, not attempting write",
master_sm->sm_id);
open_write_cb = false;
master_sm->handleEvent(CACHE_EVENT_OPEN_WRITE_FAILED, data);
Copy link
Copy Markdown
Contributor

@sudheerv sudheerv Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change this to
// the idea is to go back to read state on write fail (as opposed to relying on write_fail action)
// master_sm->handleEvent(CACHE_EVENT_OPEN_READ, data)

// make sure CACHE_EVENT_OPEN_READ in state_cache_open_write is handled without crashing (looks for stale object, in this case we may not have one)
HttpSM::state_cache_open_write(int event, void *data)

case CACHE_EVENT_OPEN_READ:
// The write vector was locked and the cache_sm retried
// and got the read vector again.
if (t_state.cache_info.object_read) {
cache_sm.cache_read_vc->get_http_info(&t_state.cache_info.object_read);
// ToDo: Should support other levels of cache hits here, but the cache does not support it (yet)
if (cache_sm.cache_read_vc->is_ram_cache_hit()) {
t_state.cache_info.hit_miss_code = SQUID_HIT_RAM;
} else {
t_state.cache_info.hit_miss_code = SQUID_HIT_DISK;
}

ink_assert(t_state.cache_info.object_read != nullptr);
t_state.source = HttpTransact::SOURCE_CACHE;
**}**
// clear up CACHE_LOOKUP_MISS, let Freshness function decide
// hit status
t_state.cache_lookup_result         = HttpTransact::CACHE_LOOKUP_NONE;
t_state.cache_info.write_lock_state = HttpTransact::CACHE_WL_READ_RETRY;
break;

@ezelkow1
Copy link
Copy Markdown
Member Author

open_write_fail.txt

Should try this patch, missed some state updates in the cache_open_read event for write_lock

This adds a new write fail action, CACHE_WL_FAIL_READ_RETRY. If it fails to obtain a write lock then it will loop for max_open_read_retries, each time doing a cache read.  In the end if it fails to ever either obtain a write lock or find the object to read then it does the default write fail action of going to the origin.

On an origin fail it will do the default of sending back an ERR_CONNECT_FAIL
@ezelkow1 ezelkow1 force-pushed the cf_core branch 2 times, most recently from cc4ee5c to a7509ed Compare October 11, 2019 19:14
ezelkow1 and others added 2 commits October 11, 2019 19:15
@bryancall
Copy link
Copy Markdown
Contributor

[approve ci]

@ezelkow1
Copy link
Copy Markdown
Member Author

superseded by #6028 since @sudheerv had a chance to work on it more

@ezelkow1 ezelkow1 closed this Oct 18, 2019
@zwoop zwoop modified the milestones: 10.0.0, QUIC Nov 4, 2019
@ezelkow1 ezelkow1 deleted the cf_core branch April 3, 2020 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants