Replies: 8 comments 28 replies
-
@tom95858 one of the items from the https://github.com/ovis-hpc/ovis-wiki/wiki/2021.03.29 notes says:
Can you elaborate on what that means? What does "dynamically" mean in this context? While an ldmsd daemon is running? And if so, how do we expect to be able to take advantage of new APIs introduced by a new plugin type? |
Beta Was this translation helpful? Give feedback.
-
The diagram at the top of https://github.com/ovis-hpc/ovis/blob/master/ldms/src/ldmsd/ldmsd-sampler-dev.md does not seem to be correct. It claims that "inst extension" contains a struct "ldmsd_plugin_inst_s", which has a "base" entry that points to an ldmsd_plugin_type_s struct. But the code later on does not match that. The code shows that base is the name of the ldmsd_plugin_inst_s inside of the "inst extension" struct, and I don't see any pointers to an lmdsd_plugin_type_s in the example code. |
Beta Was this translation helpful? Give feedback.
-
We should not have functions named "new()". It is both too generic to be easily grep-able, and it is also a keyword in C++. |
Beta Was this translation helpful? Give feedback.
-
I am still not a fan of this API approach where every plugin author needs to bundle all API functions into a struct. I think that just makes our lives much harder in the future should we ever want to extend the API. We can make this all easier on plugin authors by just making a header (probably a set of headers) with function declarations that a plugin author defines. We would want to use a function name prefix that is common across all plugin APIs. I am going to got with "ovis_plugin_" in my examples, but we could choose something else. So I would envision there being a "ovis_plugin.h" header that declares all of the functions that are common to all plugins. For instance:
In the current design, init, del, and config would be in this header too. But perhaps some of that will change as we go through the design process. Next, we would have headers for each plugin type:
(I am keeping those functions vague for now. I want to focus on the general structural approach at the moment rather the details.) This also helps move in a direction of logically separating the APIs that plugins export from the APIs that ldmsd exports to the samplers. As the document stands, those things seem a little overly integrated in the current documents. I am finding it more difficult than I think it needs to be to decode what should be used where. |
Beta Was this translation helpful? Give feedback.
-
Next, lets discuss cerate_schema() and create_set(). Why are we bunding these functions up into a struct of pointers? What value is this providing over just having the library export well-named functions? I think there is insufficient description of the semantics and goals of this struct to currently justify its existence. Which isn't to say that there aren't reasons, but those reasons will not be clear to plugin authors as it currently stands. |
Beta Was this translation helpful? Give feedback.
-
@tom95858 , @narategithub , it would be good for you guys to subscribe to this Discussion. |
Beta Was this translation helpful? Give feedback.
-
On Mon, Apr 12, 2021 at 3:49 PM Christopher J. Morrone < ***@***.***> wrote:
@narategithub <https://github.com/narategithub>, I am adding you to this
discussion because you are the expert.
I get the model that you are talking about and have used it before, the
problem is that it does not handle sub-classing. The point of the
implementation I think is to allow any function to be overridden. If you
didn't include this in the instance data, then these functions become
immutable.
It is not really clear to me that this solution *really* support
sub-classing. Just replacing a function call with another one does not
count as subclassing. And I am not at all clear on whether this approach of
function replacement is sufficient to be terribly usable in the real world.
A key feature of classes and subclassing is that we can still call the
method in the parent class while also extending that functionality. Can we
reasonably do that here?
The short answer is "yes." Although you may not like the mechanics.
For instance, how would a plugin author write their own "sample()" method?
By setting the instance.sample = my_sample, but I think you mean, how would
it call the base class implementation? That, I believe, is by calling
instance->base->sample()...
Some of the sample() functionality needs to come from the parent class, and
some from the plugin itself. How do the two functions coordinate who calls
the transaction begin and end function?
Yes, this is a good question. You want the parent to do
"transaction-start", "do -some-stuff", then "child-does-stuff", then parent
does "transaction-end." I don't think we have that. But if we want it, and
perhaps we do, then we need to change the API so we have a sample_begin(),
and sample_end()
For instance, I think one of the things we would hope to get with
subclassing is the ability to eliminate sampler_base and make that baked
into ldms.
It's not baked into LDMS, it's baked into the samper-type plugin.
Can you show me how I would practically do that with this solution?
Replacing the the "config" function from the parent "class" is not
sufficient. I can, and already have to, do that now. What I want is a
configurable config where I can tell it which things it should handle and
which it should not
Hmm...not sure about this one. I would think that if you didn't like what
the parent is doing, you have to replace it. BTW, I don't think there is
one sampler type. I think there are at least two:
- I care about job sampler type
- I don't care about jobs sampler type
Tom
… .
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#675 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABVTPXGDTMIVTPDZONHUBHLTINMEBANCNFSM42AFCKCA>
.
--
Thomas Tucker, President, Open Grid Computing, Inc.
|
Beta Was this translation helpful? Give feedback.
-
I think you need both use-cases, and goals. The use-cases address what
functionality we are trying to deliver. The goals address how we might like
to get there.
Example Use Cases:
- /sys/fs sampler
- /proc sampler
Example Goals:
- Most sampler logic is inherited from a base class
- Allow the flexibility to address complex samplers (e.g. aries_rtr)
- Writing a "/proc sampler requires only the code necessary to parse
the proc-file specific syntax
- Writing a /sys/fs sampler requires only the code that binds /sys/fs
files to metric names and types
…On Tue, Apr 13, 2021 at 2:55 PM Christopher J. Morrone < ***@***.***> wrote:
I might just argue that "base class mutability" sounds like a solution
rather than a use case. I am keeping an open mind, but I am not sure that
the complexity of inheritance is really addressing use cases that couldn't
be solved in simpler ways.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#675 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABVTPXFRZO5JL3GFJNUC5M3TISOUXANCNFSM42AFCKCA>
.
--
Thomas Tucker, President, Open Grid Computing, Inc.
|
Beta Was this translation helpful? Give feedback.
-
Stub for new api chat. See also:
https://github.com/ovis-hpc/ovis-wiki/wiki/2021.03.29
Beta Was this translation helpful? Give feedback.
All reactions