I think you should change “pbs_rsub: Reservation cannot be created from a reservation job” to “pbs_rsub: Reservation may not be created from a job already within a reservation”. I would caution your use of “can” and “may”. If something is impossible, it “cannot” be done. If a restriction has been imposed it “may not” be done.
What type of attribute is create_resv_from? Is it numeric, string, etc.?
I only have two comments. First is what is the permissions on create_resv_from? If it is for normal users, then it might be an end run around the reservation ACLs. If it is for normal users, you should check if the user is in the reservation ACLs and reject the job if not.
My other comment is that you might change create_resv_from to create_resv_from_job. The existing attribute seems to end all of a sudden.
What happens if a job is submitted into a reservation queue with “create_resv_from” attribute set to 1? I assume server will reject it but you may want to add that to design.
Walltime of the reservation will be the walltime left out of the running job or the full walltime the job was submitted for? I assume it is going to be the walltime left on the running job but it will help if you mention that.
Can jobs be altered to set “create_resv_from” attribute while they are running?
I am with @bhroam on renaming the attribute name from “create_resv_from” to “create_resv_from_job”.
What error is thrown when the job and the reservation’s user do not match?
The design document currently mentions “Interface 4” twice, please change that.
Existing functionality - if one user is submitting a job to another’ reservation - it will be reported as an Unauthorized request
other way round - new functionality - creating a reservation out of another user’s job will also display the same message - updated the design with this information.
Now, this is something I did not think of. @scc, I am with @arungrover on this, could you please let us know what according to you would be the right thing to do?
Thanks @prakashcv13 for addressing the comments. I have a few questions about the implementation though and it would be nice if you could add some of the internal design details to the document as well.
I saw the example you have listed in “hook demo.txt” file and saw that the job out of which the reservation was made was a job without a walltime. I think we should not allow such jobs to have a reservations created out of them. The reason being it is hard to decide the duration of the reservation.
There could be jobs submitted with soft walltime and in those case reservations should only use hard walltime of the job to decide its duration.
Currently we do not allow running jobs to move queues, How are you planning to move the running job into reservation queue in this case? Would it dequeue the job from its original queue and then enqueue the job into the reservation queue? If so, how will it affect limits? wouldn’t it open a way to game the system for users to move their running jobs into reservations and not get affected by run limits?
You might want to change Interface 7. It shows a qsub error for pbs_rsub command.
Not difficult, the reservation will be for 5 years.
good point to be mentioned in the design. done.
Yes, dequeue-ing and enqueue-ing is what I was thinking of. User will not be able to go around the limits if they are applied at the server/complex level.
But, if applied at the queue level, they would be able to.
I don’t think using the default scheduler duration is the right answer here. I think you should deny converting a job into a reservation unless it has a walltime. If we want to come back to this in the future, we’ll have more flexibility of how we want to implement it.
This will also screw with defaults. Any resource that was gained from a default will be lost when it is dequeued from the original queue. The resources_max on the reservation queue might replace it though. This should be tested.
I believe we actually should create the reservation from a job with no walltime request and have it be for a 5 year duration. The intent is that this will play nicely with the other feature you are working on: Deleting idle reservations.
for limits - the user can even now get passed them - by moving the job to a reservation.
for default resources - this is a documented behaviour - we should document that the default resources might get lost OR - would you suggest using the same default resources from the queue to which the job is submitted while creating a reservation queue?
The reservation will be created with the resources the job requested. A resources_max will be set on the reservation queue. Jobs pick this up as a default if there isn’t a default set. My concern might not be warranted. Could you test this? Submit a job into a queue with resources_default.foo = 5. Convert that job into a reservation. Check to see if it still has a request of foo.