PP-425 to PP-434 - Server Periodic hooks support


#21

On interface four I thought I saw run_periodic_hook instead of run periodic hook. Sorry for the confusion


#22

The server periodic hook EDD is looking good so far. Here are my comments:

interface 10: “true” should be “True” which is the Python value.

interface 13 shows something like:
j1 = pbs.job()
r1 = pbs.resv()
r1.reserve_start = int(time.time())+30
r1.reserve_duration = 600
pbs.server().submit(r1)
j1.queue = r1


Problem here is The ‘queue’ attribute of job ‘j1’ is a pbs.queue type which is incompatible with ‘r1’ which is of pbs.resv type. Might be best to create a job function that associates a job to the reservation:

j1.add_to_reservation_queue(r1)

NOTE: or whatever a good function name. I see the above call would result in submitting job ‘j1’ to the instantiated queue of reservation ‘r1’.


#23

Thanks for reviewing Al!

I was hoping to somehow overload operator in queue object to accept the reservation as an input or do this using type identification. But, You are right that this is the way it has been designed for and I’ve not very sure at the code level to see how much of that is possible. It is just how I imagined the interface to be :slight_smile:

So, I’ve changed the document to expose a method that can assign job to a reservation.
Please have a look.


#24

Looks good, Arun! Thanks for making the change.


#25

Regarding Interface 13 (version v. 20 in Confluence Page History):

I don’t think adding a new interface with a new name is warranted (or good). Rather than the new “add_to_resv()” method, I feel it is much more natural to “submit a job to a reservation” (as one uses qsub to submit jobs to reservations already.)

I suggest the method name should be “submit”. If an actual queue object is needed, the reservation object could expose its queue object (e.g., r1.queue).


#26

What Bill suggested is fine with me too.
Another option is doing it from the reservation object, via an “add” method from the reservation object. For example:

j1 = pbs.job()

r1 = pbs.resv()
r1.reserve_start = int(time.time())+30
r1.reserve_duration = 600
r1.add(j1) <-- add a job to the reservation
pbs.server().submit(r1)
pbs.server.submit(j1)


But I’m fine in either case: j1.submit() or r1.add(j1)


#27

Thanks @bayucan!

I was proposing that we do not invent a new method name, and “add()” would again be new. I think it is natural to simply use this existing submit() method name. So, to create a new job and submit it to a queue, one would:

j1 = pbs.job()
j1.queue = pbs.server().queue(“workq”)
pbs.server().submit(j1)

And to create a new reservation and submit a new job it it, one would:

r1 = pbs.resv()
r1.reserve_start = int(time.time())+30
r1.reserve_duration = 600
pbs.server().submit(r1)

j1 = pbs.job()
j1.queue = r1 // Alternatively, if we don’t want automatic type conversion, you could require: j1.queue = r1.Queue
pbs.server().submit(j1)


#28

@billnitzberg : Ok, I’m fine with what you propose. In regards to submitting a job to a reservation, I prefer the ‘j1.queue = r1.queue’…


#29

@bayucan, @billnitzberg I’ve made change to EDD as per your suggestions.


#30

@arungrover: I’m satisfied with the latest changes.


#31

Looks good to me. I’m satisfied with the latest changes


#32

Listing down all the comments from Confluence page to forum. These comments are old and may not really relate to the latest discussion but for safekeeping I’m copying it here.

From: Al Bayucan
In interface 4, be sure to add that calling pbs.event().reject() will not stop the server periodic hook from executing in the next frequency cycle.

In interface 5, you don’t need to create interface “create_node”, “createreservation”, "scheduling". You can just make use of existing: pbs.server().vnode(<new_vnode_name>), pbs.server().resv(<new_reservation>), and something like pbs.scheduling_cyle(). Note that we current have something like pbs.scheduler_restart_cycle().
Oct 08, 2016

From: Al Bayucan
Or you can actually do something like pbs.server().set_scheduling(True|False)
Oct 08, 2016

From: Al Bayucan
Same with interface 8, just make use of existing pbs.server().vnode() for defining a new node. Node here is just the natural vnode.
Oct 08, 2016

From: Al Bayucan
So the interface could look like:
vn = pbs.server().vnode(<new_vnode_name>) ← return a new vnode object to hang off of local pbs server.

Now populate the vnode object with values:
vn.resources_avaliable[] =

Same with reservation:
rv = pbs.server().resv() ← returns an reservation object to hang off the local pbs server.

Populate the reservation object:
rv.Resource_List[] =
Oct 08, 2016

From: Arun Grover
Thanks for all your comments AL!
I’ve made changes to Interface 4 and changed the interface scheduling to set_scheduling.
I added new create_vnode, create_reservation interfaces specifically because pbs.server().vnode() can also be used to search for an existing vnode and if the vnode is not existing it can throw an exception/error. But if we use it to create a vnode as well then it will never throw an error thinking that it’s been called for a new vnode.
What do you think?
Oct 10, 2016

From: Jon Shelley
Arun,
I agree with your assessment and would not want to lose the capability of throwing an error if the object (vnode in this case) does not exist. However, I would propose that we don’t use create* for each object that we want to add to the server. Instead, I would think that if we added two new methods called pbs.server().add() and pbs.server().remove(). These methods could add/remove a node, job, queue, reservation, custom resource, etc object to the server. This would allow us to present two simple methods to add and remove objects from the server. Thoughts?_
Oct 10, 2016

From: Jon Shelley
In interface 5 you do the following to get a vnode
"n1 = pbs.server().vnode()"
I would recommend that we do something like this instead
"n1 = pbs.vnode()"
Oct 11, 2016

From: Al Bayucan
I like what Jon has proposed. We actually discussed it together. There’ll be some code adjustment needed to allow calling pbs.vnode(<new_node_name>), but it should be a straightforward fix.
Oct 11, 2016

From: Arun Grover
I’m okay to have it in the way you have suggested. But, what is that we get out of it is what I need to understand.
I kept it under server because server is the one which is managing all the nodes inside of it, so it looked to me that it was logically correct to have it under server.
Who knows tomorrow one might just like a capability to search a vnode form a different server altogether (smile)
Oct 11, 2016

From: Jon Shelley
pbs.server().vnode() would be a vnode object from a server. pbs.vnode() would be a vnode object that has not yet been attached to the server. If we wanted to somehow down the road have a hook query another pbs server then I would imagine that we would do it something like pbs.server(“server name”). But I don’t see this as a very likely scenario at the moment but like you said it may be something we do down the road.