[
https://issues.apache.org/jira/browse/TS-2657?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13958278#comment-13958278
]
ASF subversion and git services commented on TS-2657:
-----------------------------------------------------
Commit 7bac4a99c1ad86b1226df84aa497ff11b9b623a7 in trafficserver's branch
refs/heads/master from [~zwoop]
[ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=7bac4a9 ]
TS-2657 Eliminate TSHttpTxnSetHttpRetBody
This patch does the following:
1. First, both TSHttpTxnSetHttpRetBody() and TSHttpTxnGetMaxHttpRetBodySize()
are removed. I’ve patched all “core” plugins to use the
TSHttpTxnErrorBodySet() API, but any non-Apache plugin would break. Hence,
this change is going in for v5.0.0, which allows breaking compatibility.
2. More importantly, TSHttpTxnErrorBodySet() requires for the body to be heap
allocated, whereas TSHttpTxnSetHttpRetBody() used a fixed size (2KB) array on
the HttpSM. This was the primary reason why I started looking into this, since
this “simple” change reduces the size of the HttpSM by 25%.
3. In order to make the TSHttpTxnErrorBodySet() API easier to use, I’ve
made the content type value NULL imply a default of "text/html". This is by
far the most common use case, and saves a good chunk of allocations.
4. The TSHttpTxnErrorBodySet() also imposes the body to be sent through the
body factory, for log field expansion. For starters, this is incredibly
inefficient , as well as imposing an 8KB size limit. This is something I
intend to ameliorate soon by improvements to the body factory. However, this
is an unnecessary inefficiency IMO; As part of the body factory improvements,
I also plan on exposing the body factory “log field” expansion as a generic
API. This would allow any plugin to decide if they want to go through the
pains of body factory expansions or not (and not limited to just error pages,
e.g. Header Rewrite would use this new API).
> Eliminate TSHttpTxnSetHttpRetBody() and improve upon TSHttpTxnErrorBodySet()
> ----------------------------------------------------------------------------
>
> Key: TS-2657
> URL: https://issues.apache.org/jira/browse/TS-2657
> Project: Traffic Server
> Issue Type: Improvement
> Components: TS API
> Reporter: Leif Hedstrom
> Assignee: Leif Hedstrom
> Labels: api-change
> Fix For: 5.0.0
>
> Attachments: TS-2657.diff
>
>
> We currently have two APIs to set an “error” body. One was added at Yahoo
> many years ago, and it was done for the wrong reasons (premature optimization
> being one of them). I’ve made a patch that unifies these two APIs into one,
> such that you can use TSHttpTxnErrorBodySet() anywhere you could use either
> of those other two APIs. There are some subtle changes in how the APIs work,
> which I’ll describe here.
> 1. First, both TSHttpTxnSetHttpRetBody() and TSHttpTxnGetMaxHttpRetBodySize()
> are removed. I’ve patched all “core” plugins to use the
> TSHttpTxnErrorBodySet() API, but any non-Apache plugin would break. Hence,
> this change is going in for v5.0.0, which allows breaking compatibility.
> 2. More importantly, TSHttpTxnErrorBodySet() requires for the body to be heap
> allocated, whereas TSHttpTxnSetHttpRetBody() used a fixed size (2KB) array on
> the HttpSM. This was the primary reason why I started looking into this,
> since this “simple” change reduces the size of the HttpSM by 25%.
> 3. In order to make the TSHttpTxnErrorBodySet() API easier to use, I’ve
> changed the content type argument to not transfer ownership. The common
> (well, exclusive) pattern using this API used to be
> TSHttpTxnErrorBodySet(txnp, body, body_len, TSstrdup(“plain/html”));
> This just seems like an overly generalized case, for a problem that doesn’t
> exist (clearly we can all manage some static strings ;). The new API is
> simply:
> TSHttpTxnErrorBodySet(txnp, body, body_len, “plain/html”);
> 4. The TSHttpTxnErrorBodySet() also imposes the body to be sent through the
> body factory, for log field expansion. For starters, this is incredibly
> inefficient , as well as imposing an 8KB size limit. This is something I
> intend to ameliorate soon by improvements to the body factory. However, this
> is an unnecessary inefficiency IMO; As part of the body factory improvements,
> I also plan on exposing the body factory “log field” expansion as a generic
> API. This would allow any plugin to decide if they want to go through the
> pains of body factory expansions or not (and not limited to just error pages,
> e.g. Header Rewrite would use this new API).
> So, I’d like to open this up for discussions and concerns. I think 3) and 4)
> are the primary candidates for concerns and objections. I’m willing to
> compromise either or both if there are major concerns (for example, is it
> worth the “cost” of changing the API in 3) ? I think it is, but I can easily
> be persuaded otherwise).
--
This message was sent by Atlassian JIRA
(v6.2#6252)
