job.submit-nocreate RPC doesn't set job state to submitted

[grondo]

As noted by @SteVwonder in flux-framework/flux-sched#304, the job.submit-nocreate RPC doesn't update the state of the job in the KVS before emitting the submitted event.

The job.submit-nocreate RPC was added as a convenience for wreckrun to attempt submitting a job in the reserved state, since the existing job.submit only worked for jobs that had not been created yet. As with job.submit, job.submit-nocreate will fail if a sched. service is not loaded, so then wreckrun can fall back to existing behavior of create a "fake" allocation.

The current behavior of emitting a wreck.state.submitted event for a job in the "reserved" state may be confusing, so should probably be fixed. However, since sched and the simulator have had to workaround the current behavior, and this instance of the job module will soon be replaced, I leave it up to @SteVwonder and @dongahn to determine if it should actually be fixed.

[dongahn]

Even if submitted event is emitted, I have a logic in JSC so this should work. But sine this logic will be reworked, I wouldn't bother. @SteVwonder?

[dongahn]

On a second thought, it seems @SteVwonder relies on the fact that submitted is not emitted. Another reason not to bother IMHO.

[garlick]

If you don't mind I think it would be good to keep this open until we're done messing with the wreck prototype, as it may be useful/easy to fix this if we are making other related changes.

[dongahn]

Sounds good. Need to hear from @SteVwonder though.

[SteVwonder]

The simulator relies on submitted not being emitted at first, but in order for the job to actually make its way into the scheduler's queue, the submitted event does eventually need to be emitted. The part I am unsure about is if the simulator (or any other piece of Flux) relies on the submitted state to be set in the KVS (although it would certainly make sense to keep the job's actual state and its state in the KVS consistent).

I agree with @dongahn that if the logic is going to be reworked, its not worth the effort to refactor anything right now.

[grondo]

Thanks for weighing in @dongahn, @SteVwonder. If everything works as is now, then I suppose nothing at this time requires state = submitted to be set in the KVS, but I worried about confusing other developers for the short time we're hacking on the wreck prototype.

It is a simple change to update the state in the KVS in the job module for the submit-nocreate RPC. I'm sorry for missing that when I threw together that PR

[SteVwonder]

@grondo: I said "it is not worth the effort to refactor", but what I really meant was "I am fine with not refactoring". Sorry for not being precise the first time.

If you think it is important to change, then I'm all for it. Let me know how I can help.

[dongahn]

Sounds good across the board, in particular changing this won't break anything.

[grondo]

Sorry @SteVwonder, I just meant I agreed with you, but wanted to add my one or two caveats...

…

On Mon, Apr 2, 2018, 8:32 AM Stephen Herbein ***@***.***> wrote: @grondo <https://github.com/grondo>: I said "it is not worth the effort to refactor", but what I really meant was "I am fine with not refactoring". Sorry for not being precise the first time. If you think it is important to change, then I'm all for it. Let me know how I can help. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1410 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAtSUraiaSl88sFseFn4uBFwzexyP7clks5tkkRygaJpZM4TC3EV> .

[grondo]

closed by #1988