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

Integrate Job Status and Control (JSC) interface #202

Closed
wants to merge 3 commits into from
Closed

Integrate Job Status and Control (JSC) interface #202

wants to merge 3 commits into from

Conversation

dongahn
Copy link
Member

@dongahn dongahn commented May 23, 2015

Provide a high-level abstraction to allow monitoring and controlling
of the status of Flux jobs. Designed to expose the job status and
control abstraction in a way to hide the underlying data layout of
job information stored within Flux's KVS data store.
Expect that schedulers and runtime tools will be its main users. This
abstraction provides the producers of job information including
a task and program execution service module including wreckrun with
an opportunity to change and optimize the data layout of jobs within
the KVS without presenting major impacts to the implementation
of the schedulers and runtime tools.

The main design considerations are the following:

  • Abstract out implementation-dependent job states;
  • Provide flexible and easily extensible mechanisms to pass job info;
  • Use a minimalistic API set.

Build on Job Control Block (JCB):
Our data schema containing the information needed to manage a
particular job. Contain information such as jobid, resources owned
by the job, as well as the processes spawned by the job. JSC converts
the raw information on a job into an JCB, implemented as an JSON
dictionary object. The current JCB structure is described in tables
in README.md.

Provide three main calls of JSC:

  • jsc_notify_status () Register a callback to the asynchronous
    status change notification service. callback will be invoked
    when the state of a job changes;
  • jsc_query_jcb () Query the key attribute of JCB of jobid;
  • jsc_update_jcb () Update the key attribute within JCB of jobid.

Provide standalone utility and testers:
Created an utility command to facilitate testing: flux-jstat. Its
usage is the following:

Usage: flux-jstat notify
flux-jstat query jobid
flux-jstat update jobid

flux-core/t/t2001-jsc.t also contains various test cases that use
this utility.

Provide a high-level abstraction to allow monitoring and controlling
of the status of Flux jobs. Designed to expose the job status and
control abstraction in a way to hide the underlying data layout of
job information stored within Flux's KVS data store.
Expect that schedulers and runtime tools will be its main users. This
abstraction provides the producers of job information including
a task and program execution service module including wreckrun with
an opportunity to change and optimize the data layout of jobs within
the KVS without presenting major impacts to the implementation
of the schedulers and runtime tools.

The main design considerations are the following:
   - Abstract out implementation-dependent job states;
   - Provide flexible and easily extensible mechanisms to pass job info;
   - Use a minimalistic API set.

Build on Job Control Block (JCB):
Our data schema containing the information needed to manage a
particular job. Contains information such as jobid, resources owned
by the job, as well as the processes spawned by the job. JSC converts
the raw information on a job into an JCB, implemented as an JSON
dictionary object. The current JCB structure is described in tables
in README.md.

Provide three main calls of JSC:
   - jsc_notify_status () Register a callback to the asynchronous
        status change notification service. callback will be invoked
        when the state of a job changes;
   - jsc_query_jcb () Query the key attribute of JCB of jobid;
   - jsc_update_jcb () updates the key attribute within JCB of jobid.

Provide standalone utility and testers:
Created an utility command to facilitate testing: flux-jstat. Its
usage is the following:

   Usage: flux-jstat notify
        flux-jstat query jobid <top level JCB attribute>
        flux-jstat update jobid <top level JCB attribute> <JCB JSON>

flux-core/t/t2001-jsc.t also contains various test cases that use
this utility.
@dongahn dongahn self-assigned this May 23, 2015
@garlick garlick added the review label May 23, 2015
@dongahn
Copy link
Member Author

dongahn commented May 23, 2015

@garlick is there a way to hear from Travis CI before creating a pull request like this?

@grondo
Copy link
Contributor

grondo commented May 23, 2015

@dongahn, you can sign up for an account at travis-ci.org and then
subscribe your personal forks of flux-core and flux-sched to automated
travis builds. You can then wait for build results on your branch before
making a pull request.

Actually I think travis-ci.org can use your github credentials so you don't
even have to create a new account. (Check for a "sign in with github"
button in the upper right)

On May 22, 2015 6:36 PM, "Dong H. Ahn" [email protected] wrote:

@garlick https://github.com/garlick is there a way to hear from Travis
CI before creating a pull request like this?


Reply to this email directly or view it on GitHub
#202 (comment)
.

@garlick
Copy link
Member

garlick commented May 23, 2015

@dongahn: this is nice work. A couple of review comments after a superficial skim:

It would be good to split this commit into (minimally):

  • add libjsc
  • add flux-jstat
  • add test case

Your API includes json-c json_object parameters. As discussed in issue #169 and begun in PR #189, we are actively replacing parameters in public facing functions with const char *json_str in Flux API so that the caller isn't forced to adopt a particular JSON implementation in their application. Probably this API should follow suit?

@grondo
Copy link
Contributor

grondo commented May 23, 2015

Good suggestions by @garlick. Also when you rebase your commits you might as well fold in your travis fixes.

One other very minor thing is leading whitespace in src/cmd/Makefile.am needs a fixup

@grondo
Copy link
Contributor

grondo commented May 23, 2015

Oh, also can you split the addition to src/common/libutil/shortjson.h into its own commit?

@dongahn
Copy link
Member Author

dongahn commented May 23, 2015

All nice suggestions! @garlick and @grondo.

I will split this PR into multiple ones with the suggested fixes.

But I am a bit concerned about using a json string in the jsc interfaces.

It is good in term of not creating a dependency to JSON-C. But what is the performance argument?

For the common case of using JSON-C, will a user have to convert a json_object into a string representation before passing it into an API and the API then have to recreate a json_object object from it?

This can add pretty large overheads for passing a large string like a large rdl.

Perhaps a right thing to do is to have an opaque type flux_json_t and allow the user to pass an json_object or a string as the argument to that? The API can then have an additional flag which allows the uses to tell which type?

@garlick
Copy link
Member

garlick commented May 24, 2015

@dongahn: in the general case it's just a question of where the conversion is done. The message functions and the KVS for the most part treat the user's json_object as opaque so it's easy to say that the new opaque argument is a json string, and it's up to the user to do any conversion. The cost then doesn't really change as the same conversions occur, but in different places.

Could your json_object JCB be replaced with a regular abstract type with accessors? Especially considering that the primary KVS object store value will be a json string (or maybe just a string) in the near future, not a json_object?

@dongahn
Copy link
Member Author

dongahn commented May 25, 2015

@garlick I think this is one of those things which would be best to discuss in person.

I wanted to avoid using a user defined type for JCB because the job schema is still in flux and JSON would be more flexible until the schema is set in stone.

I will swing by your office early next week.

@dongahn
Copy link
Member Author

dongahn commented May 26, 2015

OK. Spoke with @garlick and @grondo. The following will be my course of actions.

  1. Split this PR into a few smaller, more manageable topical PRs. (Also folding minor fixes into appropriate PRs.)
  2. Keep the use of json_object in the jsc interface as-is for now so that I can move onto higher-priority work items. Then, come back to the json-c dependency issue later on by using an opaque JCB type with accessor patterns.
  3. Fix the issue of long (and thus truncated) Travis outputs by adding test-long support into the JSC SHARDNESS testing script.

@grondo
Copy link
Contributor

grondo commented May 26, 2015

I don't think you need to split into multiple PRs, but maybe just a short series of testable (if possible) commits in a single PR.

@dongahn
Copy link
Member Author

dongahn commented May 26, 2015

Closing this. A new PR will be issued.

@dongahn dongahn closed this May 26, 2015
@garlick garlick removed the review label May 26, 2015
@dongahn dongahn deleted the jsc branch May 29, 2015 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants