trafficserver-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sudheer Vinukonda <sudheervinuko...@yahoo.com>
Subject Re: Remap plugin DSO reloading - enabling and disabling
Date Fri, 08 May 2020 03:00:45 GMT
 Hey Alan,
I do think "let user decide" is actually very relevant and important in this context. 
Some users (e.g us), like to keep the entire set (Core + Plugins + Config) as one unit and
deploy/manage them together. 
While it may not be flexible enough to be able to update a plugin at a time, there are several
benefits to this approach (for e.g integration tests, we have plugins that are owned by multiple
partner teams and we simply don't want to update those plugins individually without ensuring
each version of it gels well with the rest of the bundle). 
But, we could on the other hand want some plugins that we own directly, reloadable. And the
fact that static variables didn't work anymore took us by surprise mainly because we weren't
aware of it (totally our fault). Now that we understand that it is somewhat of an expected/intended
behavior, we'd make necessary adjustments (e,g updating the plugin) if we want to use the
reload feature.
So, it's not necessarily about whether the user is qualified to make a decision, rather a
matter of organizational structure and policy for how ATS is operated. We could clearly document
the behavior and provide recommendations to the users.
Does it make sense?
Thanks!

Sudheer
    On Thursday, May 7, 2020, 07:12:40 PM PDT, Alan Carroll <solidwallofcode@verizonmedia.com>
wrote:  
 
 Leif;
If the plugin can be global or remap but not both, I don't see why (2) limits anything. The
entire issue is irrelevant for such plugins, because the situation of reloading the remap
DSO but not the global cannot occur, In fact, option (3) or (4) would enable detecting this
and issuing a warning.
Approach (1) was my first thought, but I think the problem there is whether the plugin can
work as a global and a reloadable remap is a property of the plugin implementation, not any
particular remap rule. That is, for a specific plugin, there's really no choice about whether
to use "@plugin" or "@global" - the configuration must get it right or the plugin crashes.
Every time. Every rule. It is for this reason I disagree with Leif's view the user should
decide. The user's opinion is irrelevant - the plugin works in this mode, or it doesn't. And
as our friends at LinkedIn discovered, some rather basic C++ decisions (such as using static
variables) will prevent a plugin from working in this mode. On the other hand, if the plugin
uses the "User Args" feature then it can work, in which case what's the point of disabling
the DSO reload? Unless the plugin implementor is concerned about code skew between the global
and remap versions, which again the user is not qualified to decide.
My personal preference is (3), but I suspect after mysterious crashes with plugins, we will
have been happier with (4).
On Thu, May 7, 2020 at 7:42 PM Sudheer Vinukonda <sudheervinukonda@yahoo.com> wrote:

 +1 on the general idea to make the reloadability customizable per plugin.
However, I think it'd be more simple, cleaner and intuitive to not tie it to whether or not
a plugin is used both as a global and remap plugin.
In other words, approach (1) below but, instead of calling it "@global", we could add a param
which says "@reloadable=false" (the default value for "@reloadable" can be "true").
The same param can then be used, when we eventually add relodability to global plugins as
well.
Thoughts?



    On Thursday, May 7, 2020, 05:24:09 PM PDT, Alan Carroll <solidwallofcode@verizonmedia.com>
wrote:  
 
 As part of the ATS 9 upgrade, a feature was added so that remap plugins could have their
DSO reloaded. This means not just the configuration, but the implementation itself. While
very useful, this has some unfortunate side effects with plugins that are used in both a
global and remap context. To alleviate this, a configuration variable as added to disable
the feature.
Although reasonable, this is a rather heavy handed way to deal with the problem. What would
be better is the ability to reload the DSO or not on a per remap plugin basis. I have a few
ways this could be done:
1) Add the keyword "@global" to "remap.config". This would behave exactly as "@plugin" except
it would prohibit reloading of the DSO for that plugin.
2) Have the remap reload configuration check to see if the plugin is also a global plugin
and disable remap DSO reload for that plugin.
3) Add a flag to the global plugin registry information which can be set during TSPluginInit
which disables DSO reloading for that plugin, should it occur in "remap.config". This is similar
to (2) but requires a  plugin to prohibit DSO reloading. The call woud be TSPluginDSOReloadEnable(flag)
and would only be valid when called from TSPluginInit.
4) As (3), except the flag is set by default and must be cleared to enable DSO reloading in
"remap.config".
I'm willing to see if I can make this work, but I would like to have some feedback on the
preferred approach first.
  
  
Mime
View raw message