[jira] [Updated] (TS-3665) Redirect logic causing debug asserts and leaking cache_vc's
[ https://issues.apache.org/jira/browse/TS-3665?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Leif Hedstrom updated TS-3665: -- Fix Version/s: 6.0.0 > Redirect logic causing debug asserts and leaking cache_vc's > --- > > Key: TS-3665 > URL: https://issues.apache.org/jira/browse/TS-3665 > Project: Traffic Server > Issue Type: Bug > Components: Cache >Reporter: Susan Hinrichs > Fix For: 6.0.0 > > > This is related to TS-3140 and TS-3661. I spent this morning reviewing the > issue addressed by TS-3140 after the fixes for TS-3661 were put in place. > TS-3140 addresses the issue when the 301 is in cache, but I'm seeing asserts > for both 301's in cache and 301's not in cache. > My first assert was line 109 in HttpCacheSM.cc line 109, > ink_assert(cache_read_vc == NULL). I added a cache_sm.close_read() to the > HttpTransact::SM_ACTION_REDIRECT_READ: case of HttpSM::handle_api_return. > While only debug assert, if we ignore it we will reassign cache_read_vc > without freeing the previous. > I addressed this by adding cache_sm.close_read() to the > SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. > My second assert is in HttpSM::do_cache_prepare_action (line 4446 of > HttpSM.cc). Before the changes for TS-3661, it was expressing itself in > SM_ACTION_CACHE_ISSUE_WRITE case of HttpSM::cache_write_state(). In this > case, do_cache_prepare_action will open a new cache_write_vc overwriting the > original and losing the cache_vc memory. > The original fix to TS-3140 addressed this by adding a cache_sm.close_write > in the SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. But this > caused problems of TS-3661 causing the originally selected cache key to be > lost, but if you pass through this logic, I assume that the original cache > write vc will be lost anyway. [~sudheerv] and [~zwoop] does this situation > not happen in your redirect use cases? I'm afraid that I'm not following how > the original cache key is preserved in the second cache open only if the > first cache write open is not cleaned up. > My test URLs are: > curl -v --proxy localhost:80 > http://whos.amung.us/cwidget/4s62rme9/007071fecc4e.png > and > curl -v --proxy localhost:80 http://docs.trafficserver.apache.org -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (TS-3665) Redirect logic causing debug asserts and leaking cache_vc's
[ https://issues.apache.org/jira/browse/TS-3665?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Phil Sorber updated TS-3665: Backport to Version: 5.3.1 > Redirect logic causing debug asserts and leaking cache_vc's > --- > > Key: TS-3665 > URL: https://issues.apache.org/jira/browse/TS-3665 > Project: Traffic Server > Issue Type: Bug > Components: Cache >Reporter: Susan Hinrichs > Fix For: 6.0.0 > > > This is related to TS-3140 and TS-3661. I spent this morning reviewing the > issue addressed by TS-3140 after the fixes for TS-3661 were put in place. > TS-3140 addresses the issue when the 301 is in cache, but I'm seeing asserts > for both 301's in cache and 301's not in cache. > My first assert was line 109 in HttpCacheSM.cc line 109, > ink_assert(cache_read_vc == NULL). I added a cache_sm.close_read() to the > HttpTransact::SM_ACTION_REDIRECT_READ: case of HttpSM::handle_api_return. > While only debug assert, if we ignore it we will reassign cache_read_vc > without freeing the previous. > I addressed this by adding cache_sm.close_read() to the > SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. > My second assert is in HttpSM::do_cache_prepare_action (line 4446 of > HttpSM.cc). Before the changes for TS-3661, it was expressing itself in > SM_ACTION_CACHE_ISSUE_WRITE case of HttpSM::cache_write_state(). In this > case, do_cache_prepare_action will open a new cache_write_vc overwriting the > original and losing the cache_vc memory. > The original fix to TS-3140 addressed this by adding a cache_sm.close_write > in the SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. But this > caused problems of TS-3661 causing the originally selected cache key to be > lost, but if you pass through this logic, I assume that the original cache > write vc will be lost anyway. [~sudheerv] and [~zwoop] does this situation > not happen in your redirect use cases? I'm afraid that I'm not following how > the original cache key is preserved in the second cache open only if the > first cache write open is not cleaned up. > My test URLs are: > curl -v --proxy localhost:80 > http://whos.amung.us/cwidget/4s62rme9/007071fecc4e.png > and > curl -v --proxy localhost:80 http://docs.trafficserver.apache.org -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (TS-3665) Redirect logic causing debug asserts and leaking cache_vc's
[ https://issues.apache.org/jira/browse/TS-3665?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Susan Hinrichs updated TS-3665: --- Attachment: ts-3665.diff With ts-3665.diff, I added a close_read on the cache_sm in the REDIRECT_READ case. I converted the assert in the do_cache_prepare_action to a test against NULL and cache close_write. This enables both of my URLS to be fetched without asserts. > Redirect logic causing debug asserts and leaking cache_vc's > --- > > Key: TS-3665 > URL: https://issues.apache.org/jira/browse/TS-3665 > Project: Traffic Server > Issue Type: Bug > Components: Cache >Reporter: Susan Hinrichs > Fix For: 6.0.0 > > Attachments: ts-3665.diff > > > This is related to TS-3140 and TS-3661. I spent this morning reviewing the > issue addressed by TS-3140 after the fixes for TS-3661 were put in place. > TS-3140 addresses the issue when the 301 is in cache, but I'm seeing asserts > for both 301's in cache and 301's not in cache. > My first assert was line 109 in HttpCacheSM.cc line 109, > ink_assert(cache_read_vc == NULL). I added a cache_sm.close_read() to the > HttpTransact::SM_ACTION_REDIRECT_READ: case of HttpSM::handle_api_return. > While only debug assert, if we ignore it we will reassign cache_read_vc > without freeing the previous. > I addressed this by adding cache_sm.close_read() to the > SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. > My second assert is in HttpSM::do_cache_prepare_action (line 4446 of > HttpSM.cc). Before the changes for TS-3661, it was expressing itself in > SM_ACTION_CACHE_ISSUE_WRITE case of HttpSM::cache_write_state(). In this > case, do_cache_prepare_action will open a new cache_write_vc overwriting the > original and losing the cache_vc memory. > The original fix to TS-3140 addressed this by adding a cache_sm.close_write > in the SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. But this > caused problems of TS-3661 causing the originally selected cache key to be > lost, but if you pass through this logic, I assume that the original cache > write vc will be lost anyway. [~sudheerv] and [~zwoop] does this situation > not happen in your redirect use cases? I'm afraid that I'm not following how > the original cache key is preserved in the second cache open only if the > first cache write open is not cleaned up. > My test URLs are: > curl -v --proxy localhost:80 > http://whos.amung.us/cwidget/4s62rme9/007071fecc4e.png > and > curl -v --proxy localhost:80 http://docs.trafficserver.apache.org -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (TS-3665) Redirect logic causing debug asserts and leaking cache_vc's
[ https://issues.apache.org/jira/browse/TS-3665?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Susan Hinrichs updated TS-3665: --- Attachment: ts-3665-2.diff ts-3665-2.diff contains an updated patch of the cache read vc cleanup suggested by [~sudheerv]. It also updates the debug asserts to correctly test for the non null cache write vc's floating around in the follow redirect case. The cache read vc is not due to the ts-3661 fix. The cache write vc debug asserts are due to the ts-3661 fix. The fixes should definitely be pushed to master. The fix for ts-3661 should definitely be backported to 5.3.1. I think cleaning up the debug asserts should also be backported (the fix on this bug), but it is not such a big thing. I leave it for [~psudaemon] to decide. > Redirect logic causing debug asserts and leaking cache_vc's > --- > > Key: TS-3665 > URL: https://issues.apache.org/jira/browse/TS-3665 > Project: Traffic Server > Issue Type: Bug > Components: Cache >Reporter: Susan Hinrichs > Fix For: 6.0.0 > > Attachments: ts-3665-2.diff, ts-3665.diff > > > This is related to TS-3140 and TS-3661. I spent this morning reviewing the > issue addressed by TS-3140 after the fixes for TS-3661 were put in place. > TS-3140 addresses the issue when the 301 is in cache, but I'm seeing asserts > for both 301's in cache and 301's not in cache. > My first assert was line 109 in HttpCacheSM.cc line 109, > ink_assert(cache_read_vc == NULL). I added a cache_sm.close_read() to the > HttpTransact::SM_ACTION_REDIRECT_READ: case of HttpSM::handle_api_return. > While only debug assert, if we ignore it we will reassign cache_read_vc > without freeing the previous. > I addressed this by adding cache_sm.close_read() to the > SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. > My second assert is in HttpSM::do_cache_prepare_action (line 4446 of > HttpSM.cc). Before the changes for TS-3661, it was expressing itself in > SM_ACTION_CACHE_ISSUE_WRITE case of HttpSM::cache_write_state(). In this > case, do_cache_prepare_action will open a new cache_write_vc overwriting the > original and losing the cache_vc memory. > The original fix to TS-3140 addressed this by adding a cache_sm.close_write > in the SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. But this > caused problems of TS-3661 causing the originally selected cache key to be > lost, but if you pass through this logic, I assume that the original cache > write vc will be lost anyway. [~sudheerv] and [~zwoop] does this situation > not happen in your redirect use cases? I'm afraid that I'm not following how > the original cache key is preserved in the second cache open only if the > first cache write open is not cleaned up. > My test URLs are: > curl -v --proxy localhost:80 > http://whos.amung.us/cwidget/4s62rme9/007071fecc4e.png > and > curl -v --proxy localhost:80 http://docs.trafficserver.apache.org -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (TS-3665) Redirect logic causing debug asserts and leaking cache_vc's
[ https://issues.apache.org/jira/browse/TS-3665?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Phil Sorber updated TS-3665: Backport to Version: (was: 5.3.1) > Redirect logic causing debug asserts and leaking cache_vc's > --- > > Key: TS-3665 > URL: https://issues.apache.org/jira/browse/TS-3665 > Project: Traffic Server > Issue Type: Bug > Components: Cache >Reporter: Susan Hinrichs >Assignee: Susan Hinrichs > Fix For: 5.3.1, 6.0.0 > > Attachments: ts-3665-2.diff, ts-3665.diff > > > This is related to TS-3140 and TS-3661. I spent this morning reviewing the > issue addressed by TS-3140 after the fixes for TS-3661 were put in place. > TS-3140 addresses the issue when the 301 is in cache, but I'm seeing asserts > for both 301's in cache and 301's not in cache. > My first assert was line 109 in HttpCacheSM.cc line 109, > ink_assert(cache_read_vc == NULL). I added a cache_sm.close_read() to the > HttpTransact::SM_ACTION_REDIRECT_READ: case of HttpSM::handle_api_return. > While only debug assert, if we ignore it we will reassign cache_read_vc > without freeing the previous. > I addressed this by adding cache_sm.close_read() to the > SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. > My second assert is in HttpSM::do_cache_prepare_action (line 4446 of > HttpSM.cc). Before the changes for TS-3661, it was expressing itself in > SM_ACTION_CACHE_ISSUE_WRITE case of HttpSM::cache_write_state(). In this > case, do_cache_prepare_action will open a new cache_write_vc overwriting the > original and losing the cache_vc memory. > The original fix to TS-3140 addressed this by adding a cache_sm.close_write > in the SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. But this > caused problems of TS-3661 causing the originally selected cache key to be > lost, but if you pass through this logic, I assume that the original cache > write vc will be lost anyway. [~sudheerv] and [~zwoop] does this situation > not happen in your redirect use cases? I'm afraid that I'm not following how > the original cache key is preserved in the second cache open only if the > first cache write open is not cleaned up. > My test URLs are: > curl -v --proxy localhost:80 > http://whos.amung.us/cwidget/4s62rme9/007071fecc4e.png > and > curl -v --proxy localhost:80 http://docs.trafficserver.apache.org -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (TS-3665) Redirect logic causing debug asserts and leaking cache_vc's
[ https://issues.apache.org/jira/browse/TS-3665?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Phil Sorber updated TS-3665: Fix Version/s: 5.3.1 > Redirect logic causing debug asserts and leaking cache_vc's > --- > > Key: TS-3665 > URL: https://issues.apache.org/jira/browse/TS-3665 > Project: Traffic Server > Issue Type: Bug > Components: Cache >Reporter: Susan Hinrichs >Assignee: Susan Hinrichs > Fix For: 5.3.1, 6.0.0 > > Attachments: ts-3665-2.diff, ts-3665.diff > > > This is related to TS-3140 and TS-3661. I spent this morning reviewing the > issue addressed by TS-3140 after the fixes for TS-3661 were put in place. > TS-3140 addresses the issue when the 301 is in cache, but I'm seeing asserts > for both 301's in cache and 301's not in cache. > My first assert was line 109 in HttpCacheSM.cc line 109, > ink_assert(cache_read_vc == NULL). I added a cache_sm.close_read() to the > HttpTransact::SM_ACTION_REDIRECT_READ: case of HttpSM::handle_api_return. > While only debug assert, if we ignore it we will reassign cache_read_vc > without freeing the previous. > I addressed this by adding cache_sm.close_read() to the > SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. > My second assert is in HttpSM::do_cache_prepare_action (line 4446 of > HttpSM.cc). Before the changes for TS-3661, it was expressing itself in > SM_ACTION_CACHE_ISSUE_WRITE case of HttpSM::cache_write_state(). In this > case, do_cache_prepare_action will open a new cache_write_vc overwriting the > original and losing the cache_vc memory. > The original fix to TS-3140 addressed this by adding a cache_sm.close_write > in the SM_ACTION_REDIRECT_READ case of HttpSM::handle_api_return. But this > caused problems of TS-3661 causing the originally selected cache key to be > lost, but if you pass through this logic, I assume that the original cache > write vc will be lost anyway. [~sudheerv] and [~zwoop] does this situation > not happen in your redirect use cases? I'm afraid that I'm not following how > the original cache key is preserved in the second cache open only if the > first cache write open is not cleaned up. > My test URLs are: > curl -v --proxy localhost:80 > http://whos.amung.us/cwidget/4s62rme9/007071fecc4e.png > and > curl -v --proxy localhost:80 http://docs.trafficserver.apache.org -- This message was sent by Atlassian JIRA (v6.3.4#6332)