new plugin apis

[baallan]

Stub for new api chat. See also:
https://github.com/ovis-hpc/ovis-wiki/wiki/2021.03.29

[morrone]

@tom95858 one of the items from the https://github.com/ovis-hpc/ovis-wiki/wiki/2021.03.29 notes says:

Dynamically add new plugin types

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?

[morrone]

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.

[morrone]

@narategithub, I am not following that explanation. The plugin itself creates thermal_inst_t and returns it as an ldmsd_plugin_inst_t in new(). Later ldms calls the plugin's init() function and passes in ldmsd_plugin_inst_t. Is the ldmsd_plugin_inst_t returned by new() the same passed back to init()?

[narategithub]

yes.

[morrone]

Are you then saying that ldmsd is stuffing extra data into the ldmsd_plugin_inst_t between the new() and inti() calls?

[narategithub]

yes. thermal_inst.base.base (this confusing name will be changed soon), which is a pointer to ldmsd_sampler_type_s, is assigned by ldmsd.

[morrone]

So basically the subclassing is subtle, implicit, and confusing. It magically happen between calls into the plugin. It also would seem to make multiple inheritance, if not impossible, very difficult to use in practice. I think we need to rethink this. If we are going to use inheritance at all (I'm still unconvinced), the plugin should be able to clearly and explicitly subclass another class. It shouldn't happen off magically between calls into the plugin.

[morrone]

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++.

[morrone]

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:

/* ovis_plugin.h */

/* returns the name of this plugin */
const char *ovis_plugin_name();

/* return number representing plugin type */
int ovis_plugin_type();

/* return the minimum api version required to use this plugin */
int ovis_plugin_minimum_api_version();

/* return this plugin's version number. This is versioning specific to the plugin, and not related to api versioning. */
int ovis_plugin_version();

/* return a short description of this plugin */
const char *ovis_plugin_description();

/* return long form help */
const char * ovis_plugin_help();

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:

/* ovis_plugin_sampler.h */

/* NOTE to sampler plugin developers: you must read the base ovis_plugin.h header and implement all required API functions there */
#include <ovis_plugin.h>

/* probe the environment to discover components that need to be monitored by this instance.
  this will be called before the first call to ovis_plugin_sampler_sample(), and then at a configured rate to
  further discover changes from components that come and go. The caller ensures that ovis_plugin_sampler_discover()
  and ovis_plugin_sampler_sample() are not called simultaneously for the same instance. */
void ovis_plugin_sampler_discover();

void ovis_plugin_sampler_sample();

(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.

[morrone]

Expert? 😆

Like @tom95858 said, those pointer-to-functions are there to support override. And, the desc, help, init, etc are the defined interfaces for ldmsd to call. This is the same plugin interface styles found in many well-known projects like Linux kernel, libfabric, libibverbs, apache HTTPD module, etc.

I think that misses the point, though. My point is that we are combining state and interfaces into one blob, and I think there is a better way to go about that.

[morrone]

@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?

For instance, how would a plugin author write their own "sample()" method? 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?

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. 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.

[morrone]

Additonally, it looks to me like "help" it definitely not designed in a reasonably sub-classable way. It is just a blob of text. What if I have four layers of subclasses, each introducing different sets of configuration options? All that I can do is replace "help", not subclass it in any way. How is the final class supposed to know what options all of the classes above it provide, and assemble that into a reasonable message?

My argument here isn't necessarily that we need to really do the work to make this function. I'm not really convinced that reimplementing object oriented programming in C is really the best use of our time. I am just pointing out a flaw in the in idea that this API supports subclassing as designed.

[tom95858]

The sub-class either introduces logic before or after the base class logic, but not in the middle. This is true of any OO model I am aware of.

BTW, I am not adverse to making this new API C++.

[morrone]

I'm not implying doing anything in the middle.

[morrone]

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.

[morrone]

There is no more expansion of the number of functions than in the struct-based case. And the name space issue can easilly be handled with good prefix practices, which were also demonstrated in my example.

You claimed "self documenting" in your presentation, and I would argues that my approach is far more self documenting.

[tom95858]

There is no more expansion of the number of functions than in the struct-based case.

True.

And the name space issue can easily be handled with good prefix practices,

Not true in my opinion.

... which were also demonstrated in my example.

You end up with a never ending coupling between function names and presumed implementation, i.e. "data coupling." I just think that fundamentally it is bad practice; which is why I don't prefer the slurm-style interface. With that style over time you end up with many different names bound loosely to many different contexts by inferred matching rules. Why not make the mapping explicit...with a structure ... it identifies exactly which calls are meaningful in a given context. With slurm you need to look at the slurm implementation and understand it to know which calls matter in which context and on which nodes and in which context it may or may not be called.

You claimed "self documenting" in your presentation, and I would argues that my approach is far more self documenting.

I think @narategithub just meant that you could query the configuration and interfaces supported by a given plugin. I don't believe he was arguing that the design and goals would be immediately obvious to the observer.

[morrone]

You claimed "self documenting" in your presentation, and I would argues that my approach is far more self documenting.

I think @narategithub just meant that you could query the configuration and interfaces supported by a given plugin. I don't believe he was arguing that the design and goals would be immediately obvious to the observer.

Ah, OK. When I hear "self documenting" in programming context, I (and probably a lot of other programmers) are going to think:

https://en.wikipedia.org/wiki/Self-documenting_code

While we are on that topic, I think there are issues with embedding user documentation in help() in the code. This a hold over from the old api that I would personally like to avoid.

If one can statically document an plugin's API (which is not clear to me that we can if we introduce true inheritance where different classes in the inheritance tree can implement different capabilities), then I do not think we should do it in the code. This winds up forcing us to document our plugins' usage at least twice: once in the help() function and then again in the man page. Ideally those things should be nearly (entirely?) identical. In practice people tend to forget to update one or the other of the locations where they need to put the redundant documentation.

I think we should take inspiration from commands like git, where running "git help" basically just pulls up the man page. If we were to stick with static capabilities in a plugin (rather than inherited), then we could just have some build-time code to convert the man page roff into text and embed it in the plugin. Then we only have to write documentation once.

If we have inherited capabilities, then we might have to assemble the documentation some other way.

[tom95858]

I am not adverse to creating tools/practice to help us with documentation. It could be argued that the biggest weakness of this project is documentation. We are super-feature oriented...we could use some usability focus I think.

And you may be right about including these doc functions in the API, although if the implementation of these functions were generated by tools instead of copy and paste, that might be interesting. I'm thinking of Python .doc

[morrone]

Whoops, I mean a "plugin's configuration" not "plugin's API" above.

[morrone]

@tom95858 , @narategithub , it would be good for you guys to subscribe to this Discussion.

[tom95858]

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.

[morrone]

Wait, I'm not sure that "instance->base->sampler()" is even correct! instance->base is the set of function pointers that this plugin is implementing and exporting to the outside world. So the functions pointers for the parent class must necessarily be stored somewhere else.

[tom95858]

Yeah, I think we need a deep dive. I'm not sure either, and I don't think that all the use-cases are handled. @narategithub let's do another presentation for the user's group meeting (if people can stand it), and let's talk through some of the real use cases and they are handled with the new API.

[morrone]

I would propose that we schedule some conversations sooner, and more frequently than that (as needed?). I think the current standing 2 week interval is probably too long to make good progress on this topic. I expect there will be a lot of little things like this were we'll need to go off and read code for a while, and want to resume the discussion in a day or two.

And you may have hit the nail on the head with use cases. I was beginning to wonder what use cases this solution was trying to solve, and whether those were really the ones that we should be targeting.

[tom95858]

Ok, let's articulate the use cases we are considering at least for now. For example,

• Different plugin types: samplers, stores, transforms
• In each of the plugin types:
  • Base class mutability, what do I want to overload and why
    • sampler base class "sample" function for example. Who calls transaction_start/end
    • documentation
  • Multiple devices
  • Devices come and go
  • Device variants (schema variability)
• etc...

Get a reasonably sized list and then we can do a Zoom call after folks have had a day to digest.

My goal is to make meminfo 50 LOC...while still supporting complex samplers like PAPI that have multiple concurrent sets with schema that are individually configured etc...

[morrone]

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.

[tom95858]

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.

[morrone]

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: -

Absolutely. A subset of existing plugins is a good start to discovering common requirements. I'll suggest these two since they cover some somewhat differing approach from other plugings.

• dcgm
• lustre_ost

Most sampler logic is inherited from a base class

I am not convinced that this one is a good goal. I think the goal should be more like "make writing samplers with similar functionality easy". Introducing inheritance may support that goal, or it may work against that goal.