You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by jablko <gi...@git.apache.org> on 2014/02/23 19:17:48 UTC
[GitHub] trafficserver pull request: TS-2584 Fix failed assert transforming...
GitHub user jablko opened a pull request:
https://github.com/apache/trafficserver/pull/44
TS-2584 Fix failed assert transforming and caching negative responses
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jablko/trafficserver TS-2584
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/44.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #44
----
commit 113e8345b3d75444aeac2150b18829f2566a9127
Author: Jack Bates <ja...@nottheoilrig.com>
Date: 2014-02-23T17:59:46Z
TS-2584 Fix failed assert transforming and caching negative responses
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
If your project does not have this feature enabled and wishes so, or if the
feature is enabled but not working, please contact infrastructure at
infrastructure@apache.org or file a JIRA ticket with INFRA.
---
[GitHub] trafficserver pull request: TS-2584 Fix failed assert transforming...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/44#issuecomment-39381147
I'm not confident that this change is correct. The root cause of the assertion seems to be that `cache_info.transform_store` is not handled the same way as `cache_info.object_store`. In `HttpTransact::handle_transform_ready`, we know that the `transform_response` is valid, so that might be a reasonable place to set the response into `cache_info.transform_store`.
I would also like to understand why `HttpTransact::set_headers_for_cache_write` cares about caching a negative response at all. I *think* that this is all about setting up a special response to write to cache when caching a negative response and being careful not to trash that with a different response header.
Finally, it sounds like it should be simple to write a test for this in the `ci/tsqa` harness using the null transform example plugin and curl.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2584 Fix failed assert transforming...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/44#issuecomment-39865474
It seems to me there are two effecrive changes for this. The first minor one is that the response can be set earlier in the method. The more concerning one is that whether the response is set at all for negative caching. If the info is valid on entry then it won't be, but if the entry isn't set then it will be. Presumably the difference is that the transform is never valid on entry. The big question is whether setting the response can be a problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2584 Fix failed assert transforming...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/44#issuecomment-39381820
@SolidWallOfCode should review this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2584 Fix failed assert transforming...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/44#issuecomment-39675175
I will get to this this week.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2584 Fix failed assert transforming...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafficserver/pull/44
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-2584 Fix failed assert transforming...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/44#issuecomment-56890505
Can this be closed ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---