You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Sudheer Vinukonda <su...@yahoo.com.INVALID> on 2020/04/14 19:23:55 UTC
New TS API TSSyncStatFindName()
The existing TS API `TSStaFindName()` returns success if it's able to find the stat in the stats hash table. This automatically becomes true for persistent stats that get loaded from the stats snapshot, even when a `TSStatCreate()` hasn't been called yet on that stat after a server restart.
There are certain use cases, which check if `TSStatFindName()` returns not found and only then create the stat (call `TSStatCreate()`) - for example, header-rewrite plugin. The intention there is to not call `TSStatCreate` duplicate on a given counter that's potentially created via multiple rules.
However, this results in not setting the Sync CB for the counter while also not updating the api_rsb_index. This can potentially affect subsequent API stats as they end up getting stat ids that are already in use by previously used stats that did not end up calling `TSStatCreate`
To address this, I'm proposing to add a new TS API `TSSyncStatFindName()` which returns found only if the stat is in the hash table and it's sync_cb is set. This API will replace the usage of `TSStatFindName` in header-rewrite and other plugins that we internally have which have similar use cases.
Let me know any concerns/comments/feedback.
Thanks,
Sudheer
Re: New TS API TSSyncStatFindName()
Posted by Sudheer Vinukonda <su...@yahoo.com>.
Yeah, that sounds fair! I'll change the existing `TSStatFindName()` API to only return success if the found stat also has a sync callback already set. Sync Callback (e.g a Counter/Guage/Average etc) is set by calling TSStatCreate() ) and this change will ensure that `TSStatFindName()` will not return success when a stat is loaded from disk upon a restart but no TSStatCreate() is called on it yet.
This change should be mostly seamless because this feels like the expected/intended behavior, but, if anyone has concerns or need the old behavior preserved, pls let me know :)
Here's the PR
https://github.com/apache/trafficserver/pull/6658
Thanks,
Sudheer
On Tuesday, April 14, 2020, 03:24:33 PM PDT, Leif Hedstrom <zw...@apache.org> wrote:
> On Apr 14, 2020, at 1:23 PM, Sudheer Vinukonda <su...@yahoo.com> wrote:
>
> The existing TS API `TSStaFindName()` returns success if it's able to find the stat in the stats hash table. This automatically becomes true for persistent stats that get loaded from the stats snapshot, even when a `TSStatCreate()` hasn't been called yet on that stat after a server restart.
>
> There are certain use cases, which check if `TSStatFindName()` returns not found and only then create the stat (call `TSStatCreate()`) - for example, header-rewrite plugin. The intention there is to not call `TSStatCreate` duplicate on a given counter that's potentially created via multiple rules.
>
> However, this results in not setting the Sync CB for the counter while also not updating the api_rsb_index. This can potentially affect subsequent API stats as they end up getting stat ids that are already in use by previously used stats that did not end up calling `TSStatCreate`
>
> To address this, I'm proposing to add a new TS API `TSSyncStatFindName()` which returns found only if the stat is in the hash table and it's sync_cb is set. This API will replace the usage of `TSStatFindName` in header-rewrite and other plugins that we internally have which have similar use cases.
>
> Let me know any concerns/comments/feedback.
Hmmm, why not just “fix” the existing API for 9.0.0, and be done with it? Or, alternatively, if both functionalities needs to be available, add another option to the existing API(s)? I’m hesitant to add more complexity with difficult to chose from APIs, particularly since we’re right in the middle of a major (9.0.0) release right now.
— Leif
Re: New TS API TSSyncStatFindName()
Posted by Sudheer Vinukonda <su...@yahoo.com.INVALID>.
Yeah, that sounds fair! I'll change the existing `TSStatFindName()` API to only return success if the found stat also has a sync callback already set. Sync Callback (e.g a Counter/Guage/Average etc) is set by calling TSStatCreate() ) and this change will ensure that `TSStatFindName()` will not return success when a stat is loaded from disk upon a restart but no TSStatCreate() is called on it yet.
This change should be mostly seamless because this feels like the expected/intended behavior, but, if anyone has concerns or need the old behavior preserved, pls let me know :)
Here's the PR
https://github.com/apache/trafficserver/pull/6658
Thanks,
Sudheer
On Tuesday, April 14, 2020, 03:24:33 PM PDT, Leif Hedstrom <zw...@apache.org> wrote:
> On Apr 14, 2020, at 1:23 PM, Sudheer Vinukonda <su...@yahoo.com> wrote:
>
> The existing TS API `TSStaFindName()` returns success if it's able to find the stat in the stats hash table. This automatically becomes true for persistent stats that get loaded from the stats snapshot, even when a `TSStatCreate()` hasn't been called yet on that stat after a server restart.
>
> There are certain use cases, which check if `TSStatFindName()` returns not found and only then create the stat (call `TSStatCreate()`) - for example, header-rewrite plugin. The intention there is to not call `TSStatCreate` duplicate on a given counter that's potentially created via multiple rules.
>
> However, this results in not setting the Sync CB for the counter while also not updating the api_rsb_index. This can potentially affect subsequent API stats as they end up getting stat ids that are already in use by previously used stats that did not end up calling `TSStatCreate`
>
> To address this, I'm proposing to add a new TS API `TSSyncStatFindName()` which returns found only if the stat is in the hash table and it's sync_cb is set. This API will replace the usage of `TSStatFindName` in header-rewrite and other plugins that we internally have which have similar use cases.
>
> Let me know any concerns/comments/feedback.
Hmmm, why not just “fix” the existing API for 9.0.0, and be done with it? Or, alternatively, if both functionalities needs to be available, add another option to the existing API(s)? I’m hesitant to add more complexity with difficult to chose from APIs, particularly since we’re right in the middle of a major (9.0.0) release right now.
— Leif
Re: New TS API TSSyncStatFindName()
Posted by Leif Hedstrom <zw...@apache.org>.
> On Apr 14, 2020, at 1:23 PM, Sudheer Vinukonda <su...@yahoo.com> wrote:
>
> The existing TS API `TSStaFindName()` returns success if it's able to find the stat in the stats hash table. This automatically becomes true for persistent stats that get loaded from the stats snapshot, even when a `TSStatCreate()` hasn't been called yet on that stat after a server restart.
>
> There are certain use cases, which check if `TSStatFindName()` returns not found and only then create the stat (call `TSStatCreate()`) - for example, header-rewrite plugin. The intention there is to not call `TSStatCreate` duplicate on a given counter that's potentially created via multiple rules.
>
> However, this results in not setting the Sync CB for the counter while also not updating the api_rsb_index. This can potentially affect subsequent API stats as they end up getting stat ids that are already in use by previously used stats that did not end up calling `TSStatCreate`
>
> To address this, I'm proposing to add a new TS API `TSSyncStatFindName()` which returns found only if the stat is in the hash table and it's sync_cb is set. This API will replace the usage of `TSStatFindName` in header-rewrite and other plugins that we internally have which have similar use cases.
>
> Let me know any concerns/comments/feedback.
Hmmm, why not just “fix” the existing API for 9.0.0, and be done with it? Or, alternatively, if both functionalities needs to be available, add another option to the existing API(s)? I’m hesitant to add more complexity with difficult to chose from APIs, particularly since we’re right in the middle of a major (9.0.0) release right now.
— Leif
Re: New TS API TSSyncStatFindName()
Posted by Leif Hedstrom <zw...@apache.org>.
> On Apr 14, 2020, at 1:23 PM, Sudheer Vinukonda <su...@yahoo.com> wrote:
>
> The existing TS API `TSStaFindName()` returns success if it's able to find the stat in the stats hash table. This automatically becomes true for persistent stats that get loaded from the stats snapshot, even when a `TSStatCreate()` hasn't been called yet on that stat after a server restart.
>
> There are certain use cases, which check if `TSStatFindName()` returns not found and only then create the stat (call `TSStatCreate()`) - for example, header-rewrite plugin. The intention there is to not call `TSStatCreate` duplicate on a given counter that's potentially created via multiple rules.
>
> However, this results in not setting the Sync CB for the counter while also not updating the api_rsb_index. This can potentially affect subsequent API stats as they end up getting stat ids that are already in use by previously used stats that did not end up calling `TSStatCreate`
>
> To address this, I'm proposing to add a new TS API `TSSyncStatFindName()` which returns found only if the stat is in the hash table and it's sync_cb is set. This API will replace the usage of `TSStatFindName` in header-rewrite and other plugins that we internally have which have similar use cases.
>
> Let me know any concerns/comments/feedback.
Hmmm, why not just “fix” the existing API for 9.0.0, and be done with it? Or, alternatively, if both functionalities needs to be available, add another option to the existing API(s)? I’m hesitant to add more complexity with difficult to chose from APIs, particularly since we’re right in the middle of a major (9.0.0) release right now.
— Leif