You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Jeff Elsloo <el...@apache.org> on 2021/05/04 23:35:58 UTC

New TS API Proposal: TSHttpTxnCacheDiskPathGet

Hi all,

I'd like to propose the introduction of a new TS API,
TSHttpTxnCacheDiskPathGet. The function signature is as follows:

tsapi TSReturnCode TSHttpTxnCacheDiskPathGet(TSHttpTxn txnp, const char **path);

The intent of this API is to expose the underlying disk used to
service a request, regardless of whether the request is a cache hit or
miss. This is necessary because, as far as I know, there has
historically been no way to easily obtain this information on a
running system. This makes troubleshooting storage problems arduous
because unless there's a major outlier, it's very difficult to
determine which disk holds a particular object.

In terms of the API itself, it relies on `const char **` to ensure the
underlying path doesn't get modified and to avoid unnecessary copying
since this is a read-only value.

The API and all dependent changes can be seen in this PR:
https://github.com/apache/trafficserver/pull/7783

These changes are being introduced to support an enhancement to the
xdebug plugin that can be found here:
https://github.com/apache/trafficserver/pull/7784

Feedback welcome!
--
Thanks,
Jeff

Re: [E] New TS API Proposal: TSHttpTxnCacheDiskPathGet

Posted by Jeff Elsloo <el...@apache.org>.
Hi all,

Based on a brief conversion on Slack with amc, I've added the length argument.

Update commit: https://github.com/apache/trafficserver/pull/7783/commits/b44c9d932f3c4d9fdbdb037d7286d859e260bfe1

Please let me know if there is any other feedback on this API.
--
Thanks,
Jeff

On Thu, May 6, 2021 at 11:42 AM Jeff Elsloo <el...@apache.org> wrote:
>
> I just pushed a commit that incorporates this feedback and updated the
> dependent PR that enhances the xdebug plugin to reflect the change.
> The diff is as follows:
>
> – tsapi TSReturnCode TSHttpTxnCacheDiskPathGet(TSHttpTxn txnp, const
> char **path);
> + tsapi const char *TSHttpTxnCacheDiskPathGet(TSHttpTxn txnp);
>
> Update commit: https://github.com/apache/trafficserver/pull/7783/commits/020a093ab362b34647e5e42e437688ca289fd2bc
> --
> Thanks,
> Jeff
>
> On Wed, May 5, 2021 at 11:58 AM Alan Carroll
> <so...@verizonmedia.com.invalid> wrote:
> >
> > Going forward we want to be view oriented, and also to be consistent with
> > other API, the signature should be arranged to return a view. E.g.
> >
> > const char * TSHttpTxnCacheDiskPathGet(TSHttpTxn txn, int * length);
> >
> > This is modeled on API calls such as TSHttpHdrHostGet,
> > TSHttpTxnPluginTagGet, and the family of TSUrlHostGet etc.
> >
> >
> >
> > On Tue, May 4, 2021 at 6:36 PM Jeff Elsloo <el...@apache.org> wrote:
> >
> > > Hi all,
> > >
> > > I'd like to propose the introduction of a new TS API,
> > > TSHttpTxnCacheDiskPathGet. The function signature is as follows:
> > >
> > > tsapi TSReturnCode TSHttpTxnCacheDiskPathGet(TSHttpTxn txnp, const char
> > > **path);
> > >
> > > The intent of this API is to expose the underlying disk used to
> > > service a request, regardless of whether the request is a cache hit or
> > > miss. This is necessary because, as far as I know, there has
> > > historically been no way to easily obtain this information on a
> > > running system. This makes troubleshooting storage problems arduous
> > > because unless there's a major outlier, it's very difficult to
> > > determine which disk holds a particular object.
> > >
> > > In terms of the API itself, it relies on `const char **` to ensure the
> > > underlying path doesn't get modified and to avoid unnecessary copying
> > > since this is a read-only value.
> > >
> > > The API and all dependent changes can be seen in this PR:
> > >
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_trafficserver_pull_7783&d=DwIBaQ&c=sWW_bEwW_mLyN3Kx2v57Q8e-CRbmiT9yOhqES_g_wVY&r=5nE_8e-Jc1t5vF6GVeub9BCN4FzSc_6kU7_mjSiUrDs&m=aHGMIr5lOh72igF4NkBml97vttmDT5ELa4aCLhBPRE8&s=B7o7E6pQaiZsr_OMKEfp98d7ebMPlSab-SqdxQR33J0&e=
> > >
> > > These changes are being introduced to support an enhancement to the
> > > xdebug plugin that can be found here:
> > >
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_trafficserver_pull_7784&d=DwIBaQ&c=sWW_bEwW_mLyN3Kx2v57Q8e-CRbmiT9yOhqES_g_wVY&r=5nE_8e-Jc1t5vF6GVeub9BCN4FzSc_6kU7_mjSiUrDs&m=aHGMIr5lOh72igF4NkBml97vttmDT5ELa4aCLhBPRE8&s=457PabZ0MV1uUBhoIeAo-IeMHX_HL6nTYJNmjiUbX-Y&e=
> > >
> > > Feedback welcome!
> > > --
> > > Thanks,
> > > Jeff
> > >

Re: [E] New TS API Proposal: TSHttpTxnCacheDiskPathGet

Posted by Jeff Elsloo <el...@apache.org>.
I just pushed a commit that incorporates this feedback and updated the
dependent PR that enhances the xdebug plugin to reflect the change.
The diff is as follows:

– tsapi TSReturnCode TSHttpTxnCacheDiskPathGet(TSHttpTxn txnp, const
char **path);
+ tsapi const char *TSHttpTxnCacheDiskPathGet(TSHttpTxn txnp);

Update commit: https://github.com/apache/trafficserver/pull/7783/commits/020a093ab362b34647e5e42e437688ca289fd2bc
--
Thanks,
Jeff

On Wed, May 5, 2021 at 11:58 AM Alan Carroll
<so...@verizonmedia.com.invalid> wrote:
>
> Going forward we want to be view oriented, and also to be consistent with
> other API, the signature should be arranged to return a view. E.g.
>
> const char * TSHttpTxnCacheDiskPathGet(TSHttpTxn txn, int * length);
>
> This is modeled on API calls such as TSHttpHdrHostGet,
> TSHttpTxnPluginTagGet, and the family of TSUrlHostGet etc.
>
>
>
> On Tue, May 4, 2021 at 6:36 PM Jeff Elsloo <el...@apache.org> wrote:
>
> > Hi all,
> >
> > I'd like to propose the introduction of a new TS API,
> > TSHttpTxnCacheDiskPathGet. The function signature is as follows:
> >
> > tsapi TSReturnCode TSHttpTxnCacheDiskPathGet(TSHttpTxn txnp, const char
> > **path);
> >
> > The intent of this API is to expose the underlying disk used to
> > service a request, regardless of whether the request is a cache hit or
> > miss. This is necessary because, as far as I know, there has
> > historically been no way to easily obtain this information on a
> > running system. This makes troubleshooting storage problems arduous
> > because unless there's a major outlier, it's very difficult to
> > determine which disk holds a particular object.
> >
> > In terms of the API itself, it relies on `const char **` to ensure the
> > underlying path doesn't get modified and to avoid unnecessary copying
> > since this is a read-only value.
> >
> > The API and all dependent changes can be seen in this PR:
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_trafficserver_pull_7783&d=DwIBaQ&c=sWW_bEwW_mLyN3Kx2v57Q8e-CRbmiT9yOhqES_g_wVY&r=5nE_8e-Jc1t5vF6GVeub9BCN4FzSc_6kU7_mjSiUrDs&m=aHGMIr5lOh72igF4NkBml97vttmDT5ELa4aCLhBPRE8&s=B7o7E6pQaiZsr_OMKEfp98d7ebMPlSab-SqdxQR33J0&e=
> >
> > These changes are being introduced to support an enhancement to the
> > xdebug plugin that can be found here:
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_trafficserver_pull_7784&d=DwIBaQ&c=sWW_bEwW_mLyN3Kx2v57Q8e-CRbmiT9yOhqES_g_wVY&r=5nE_8e-Jc1t5vF6GVeub9BCN4FzSc_6kU7_mjSiUrDs&m=aHGMIr5lOh72igF4NkBml97vttmDT5ELa4aCLhBPRE8&s=457PabZ0MV1uUBhoIeAo-IeMHX_HL6nTYJNmjiUbX-Y&e=
> >
> > Feedback welcome!
> > --
> > Thanks,
> > Jeff
> >

Re: [E] New TS API Proposal: TSHttpTxnCacheDiskPathGet

Posted by Alan Carroll <so...@verizonmedia.com.INVALID>.
Going forward we want to be view oriented, and also to be consistent with
other API, the signature should be arranged to return a view. E.g.

const char * TSHttpTxnCacheDiskPathGet(TSHttpTxn txn, int * length);

This is modeled on API calls such as TSHttpHdrHostGet,
TSHttpTxnPluginTagGet, and the family of TSUrlHostGet etc.



On Tue, May 4, 2021 at 6:36 PM Jeff Elsloo <el...@apache.org> wrote:

> Hi all,
>
> I'd like to propose the introduction of a new TS API,
> TSHttpTxnCacheDiskPathGet. The function signature is as follows:
>
> tsapi TSReturnCode TSHttpTxnCacheDiskPathGet(TSHttpTxn txnp, const char
> **path);
>
> The intent of this API is to expose the underlying disk used to
> service a request, regardless of whether the request is a cache hit or
> miss. This is necessary because, as far as I know, there has
> historically been no way to easily obtain this information on a
> running system. This makes troubleshooting storage problems arduous
> because unless there's a major outlier, it's very difficult to
> determine which disk holds a particular object.
>
> In terms of the API itself, it relies on `const char **` to ensure the
> underlying path doesn't get modified and to avoid unnecessary copying
> since this is a read-only value.
>
> The API and all dependent changes can be seen in this PR:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_trafficserver_pull_7783&d=DwIBaQ&c=sWW_bEwW_mLyN3Kx2v57Q8e-CRbmiT9yOhqES_g_wVY&r=5nE_8e-Jc1t5vF6GVeub9BCN4FzSc_6kU7_mjSiUrDs&m=aHGMIr5lOh72igF4NkBml97vttmDT5ELa4aCLhBPRE8&s=B7o7E6pQaiZsr_OMKEfp98d7ebMPlSab-SqdxQR33J0&e=
>
> These changes are being introduced to support an enhancement to the
> xdebug plugin that can be found here:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_trafficserver_pull_7784&d=DwIBaQ&c=sWW_bEwW_mLyN3Kx2v57Q8e-CRbmiT9yOhqES_g_wVY&r=5nE_8e-Jc1t5vF6GVeub9BCN4FzSc_6kU7_mjSiUrDs&m=aHGMIr5lOh72igF4NkBml97vttmDT5ELa4aCLhBPRE8&s=457PabZ0MV1uUBhoIeAo-IeMHX_HL6nTYJNmjiUbX-Y&e=
>
> Feedback welcome!
> --
> Thanks,
> Jeff
>