Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvs: add API for RFC 18 KVS event logs #1654

Closed
garlick opened this Issue Sep 11, 2018 · 8 comments

Comments

Projects
None yet
4 participants
@garlick
Copy link
Member

garlick commented Sep 11, 2018

A standardized format for timestamped KVS event logs was proposed in RFC 18. We should consider implementing an API for this.

One suggestion was to just use the regular KVS lookup API to get (or watch) an eventlog, then have another API to iterate on and update an event log, that could be separately tested, e.g.

// create eventlog from string payload returned by flux_kvs_lookup_get()
struct flux_eventlog *flux_eventlog_decode (const char *s);

// create string from eventlog
const char *flux_eventlog_encode (struct flux_eventlog *eventlog);

struct flux_eventlog_entry {
    time_t timestamp;
    char *name;
    char *context;
};

// iterate over events in an eventlog (return NULL at end)
const struct flux_eventlog_entry *flux_eventlog_first (struct flux_eventlog *eventlog);
const struct flux_eventlog_entry *flux_eventlog_next (struct flux_eventlog *eventlog);

// update (grow) eventlog without altering cursor position
// 's' is a complete copy of the expanded eventlog, as returned from a lookup response
// a flux_eventlog_next() that returned NULL may be tried again to fetch the next entry
int flux_eventlog_update (struct flux_eventlog *eventlog, const char *s);

// add an entry to an eventlog manually (e.g. for testing)
int flux_eventlog_append (struct flux_eventlog *eventlog,
                          const struct flux_eventlog_entry *entry);
@SteVwonder

This comment has been minimized.

Copy link
Member

SteVwonder commented Sep 11, 2018

The timing on this is impeccable. @dongahn and I were just talking about this.

@dongahn: I confirmed with @garlick that the scheduler (and other modules) can append to their own individual event log by changing the key passed to flux_kvs_evlog_append.

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Sep 11, 2018

I would argue for keeping the eventlog name in the API, or renaming event logs to "evlogs". evlog only saves 3 characters...

The API might be more flexible if a flux_eventlog_t handle was retrieved from the flux_future_t returned from flux_kvs_evlog_lookup(), then it could have iterators with a nicer interface and type checking. Also, for unit tests, a flux_eventlog_t could be created from a file and the API exercised that way. The future returned by lookup might also be used to "udpate" an eventlog_t.

Main consumer use-cases of the eventlog interface will be to get notified when an event occurs, or search for a given state and its timestamp, etc. It would be a nicer interface if these high level functions didn't take a future..

@dongahn

This comment has been minimized.

Copy link
Contributor

dongahn commented Sep 12, 2018

@dongahn: I confirmed with @garlick that the scheduler (and other modules) can append to their own individual event log by changing the key passed to flux_kvs_evlog_append.

It is not directly related to this, but as you and I chatted yesterday. For the main scheduler, I really do think that we need to hide the underlying mechanisms by which the states are being traced into a higher level API.

The main scheduler should be able to make use of jobs' state changes without having to know the details of where/how the states are being tracked. It would be best to write the scheduler loop service as a finite state machine that reacts to each and all the state changes it requires without having to know whether it needs to use eventing and/or watch and/or global state log and/or local state log.

This way, the complexity of having to deal with the underlying mechanism will stay within the implementation of the API while the scheduler loop module will stay lean and mean.

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 14, 2018

@grondo: I think that's a good suggestion. I'll iterate on the API in the description of this issue until we like it.

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 14, 2018

It is not directly related to this, but as you and I chatted yesterday. For the main scheduler, I really do think that we need to hide the underlying mechanisms by which the states are being traced into a higher level API.

Yeah, and actually I think we will have a separate API to the job module for querying job states and events. This will be a lower level interface used by the job module.

@dongahn

This comment has been minimized.

Copy link
Contributor

dongahn commented Sep 14, 2018

Yeah, and actually I think we will have a separate API to the job module for querying job states and events. This will be a lower level interface used by the job module.

Sounds good. It would be also nice if we can use the same high level status/control API to access both the global event log and the local log that the scheduler will use to keep track of the more fine grained state of the jobs relevant to scheduling.

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 15, 2018

I reworked the original straw man proposal per @grondo's suggestions so that it's decoupled from the KVS lookup functions.

@dongahn - my understanding of the plan thus far is that we want to build up a generalized set of API's:

  1. for manipulating a well defined eventlog format, as defined in rfc 18 (what this issue is about)
  2. API + service in the job manager to implement the "wait one" and "wait all" interfaces, that would be open to guests (for their own jobs only). I think this could work on any eventlog associated with a job, and would not need to know anything about specific events.
  3. API for interpreting well defined event sequences in the main job eventlog

I'm the most vague on that last one, and I think it may need more discussion. The gist of it was that the main job eventlog needs to be "reduced" for consumption by user-facing tools that list the queue or report job status.

Here is where I start to get lost: user-facing tools may also need to interpret events from the scheduler eventlog, the job shell eventlog, etc.. But one goal of the multiple event logs was for a separation of concerns, so that a job shell or scheduler could be free to define events that make sense for its particular implementation. I think we may have said that there should be some well defined events for schedulers and job shells as well. But then why not just put those in the main job eventlog as they become well defined?

Rather than let this issue drift off topic, I'll go ahead and open issues on items 2 and 3 and we can continue discussion(s) there. I think the plan needs wider review and discussion, though I feel the eventlog design is pretty solid and it will be fine to continue to make progress on the lower level stuff as we sort things out.

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Sep 27, 2018

This was resolved when PR #1671 was merged.

@garlick garlick closed this Sep 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.