Hi,
I’ve created a small document on Confluence which talks about all external changes that will be visible to end users when server periodic hooks are added to PBS server.
Please go through the page and let me know if you have any comments/suggestions.
Thanks,
Arun Grover
Hey Arun,
Here are a few comments.
Interface 1: While using the single letter short form for the qmgr commands is correct, I like spelling them out. It removes any confusion if someone doesn’t know what ‘c’ or ‘i’ means. As a note, there is an extra space in qmgr -c " c h …"
Interface 2: I’d use the terminology, “restart the hook frequency” instead of “start counting the ticks of the hook frequency again”
Interface 3: This point brings up an interesting quandary with unset. I’m assuming when you unset either the frequency or alarm, they go back to the default value. If you have the alarm greater than the default frequency and unset the frequency (or vice versa), what will happen? Are you going to reject the unset command? If you don’t, you’ll end up in the situation you’re trying to avoid here.
Interface 4: There are two spaces between “by” and “”. I don’t know if that is intensional.
Interface 5: scheduling is a server attribute. Don’t those get set directly? pbs.server().scheduling = True?
Interface 8: If a node doesn’t actually get created until after the accept(), the hook writer won’t get any immediate feedback if the node was created successfully. This will make it difficult for them to create and immediately use it. They’ll just have to assume the creation was successful or wait until the next iteration of the hook.
Interface 9: When you say, “fails to create the reservation” do you mean fails to confirm or truly fails to create it?
Hi @bhroam,
Thanks for your review comments. I’ve made appropriate changes to the document as per your suggestion.
Below are the answers to some of the questions you raised:
Interface 3: This point brings up an interesting quandary with unset. I’m assuming when you unset either the frequency or alarm, they go back to the default value. If you have the alarm greater than the default frequency and unset the frequency (or vice versa), what will happen? Are you going to reject the unset command? If you don’t, you’ll end up in the situation you’re trying to avoid here.
[AG] - It’s an interesting point that you called out here. Now when I think about it there could also be a case that one sets the freq and alarm before setting the hook event type to server_periodic. So, to handle these cases, I’ve added another error string in the document. I can also just get rid of all other error strings and just keep one if that helps.
Interface 8: If a node doesn’t actually get created until after the accept(), the hook writer won’t get any immediate feedback if the node was created successfully. This will make it difficult for them to create and immediately use it. They’ll just have to assume the creation was successful or wait until the next iteration of the hook.
[AG]: That’s the limitation here, this hook runs out of process and we would not want server getting updated for every transaction. Especially when a hook writer based on some condition decides to reject all his/her changes. Well, I think this is the case with our other out of process hooks like (mom_periodic). The change only takes effect when the hook accepts. If a user really wants to take corrective action then they can write a hook which looks at the changes it has made iteration per iteration.
Interface 9: When you say, “fails to create the reservation” do you mean fails to confirm or truly fails to create it?
[AG]: Thanks for finding this out! I meant failed to confirm a reservation and not create it. Confirmation will only happen when hook accepts the changes and scheduler finds out a place to run it on.
Here is the list of early comments which I got on the confluence page (Thanks to Jon and Al for providing their comments) -
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”, “create_reservation”, “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().
Al Bayucan
Or you can actually do something like pbs.server().set_scheduling(True|False)
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.
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[] =
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?
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?
I like the idea of something like:
vn = pbs.vnode(<new_vnode_name>)
vn.resources_available[‘file’] = pbs.size(“1gb”)
pbs.server().add(vn) <-- add a new node/vnode to local server, doing an equivalent of ‘qmgr -c “create node <node_name>”’
Similarly:
resv = pbs.resv()
resv.resources_available[“file”] = pbs.size(“2gb”)
pbs.server().add(resv) <-- add a new reservation.
Extending this, one can go:
que = pbs.queue(<new_queue_name>)
que.enabled=True
que.started=True
que.type = pbs.EXECUTION
pbs.server().add(que) <-- add a new queue.
I would think that it would be better to execute the pbs.server().add() and remove() call when it is called vs when the hook is accepted. This allows for the hook writer to know if it succeeds or fails. If the hook writer calls a pbs.event().reject() then they are responsible to back out the changes before they reject the hook. I believe this is done in other places in the hook infrastructure but Al would know for sure.
Thinking about this more the only purpose of calling reject or accept in a periodic server hook would be to back out of any pbs side changes without keeping track of what changes were made. I’m not sure how often this would be used and to remove the feedback from the hook writer seems like a big price to pay for the back out capability.
A few comments/suggestions… a design meeting where the draft design could be presented with the new interfaces and allow for open feedback and interaction would be an efficient way to get good feedback and suggestions. (You could post the meeting info here, for example, to allow anyone to participate?)
Here are a few comments/suggestions…
I like the use of the existing pbs.vnode, pbs.queue, pbs.resv, and pbs.job as constructors for creating new objects of these types. A further simplifying suggestion is that the design could require that the minimum set of parameters be supplied at the time the constructor is called to eliminate lots of error checking and handling, e.g., require the same minimum set of parameters to pbs.resv(xxx) as one requires for pbs_rsub xxx, or throw an exception.
Both qmgr and IFL use “create” and “delete” for adding and removing PBS server objects. Unless there is a good Python reason to use “add” and “remove”, I suggest using “create” and “delete”. Of course, that works well for vnodes and queues, but both jobs and reservations are “submitted”, so they may need further or different handling and workflows.
The other server-level hook events do not explicitly use “server” in their names. It is worth thinking about being parallel with the other names (figure 6.1 in the Admin Guide is a nice overview). Naming this hook event as “periodic” (without server_) might be more consistent.
Does anything need to be defined with respect to primary/secondary servers? It is also worth thinking about what guarantees we specifically want to avoid so that there are no unintended serialization points when we move to active-active parallel servers (in the future).
Thanks!
I totally agree with you there. But, if we move to an implementation that updates server every now and then, then that implementation will not be in sync with how mom periodic hooks operate as of today.
Initially I also thought in the same way and had a modify() action to update server whenever needed.
This can be done. We can expect a dictionary of bare minimum attributes that are needed to create these objects.
How about a create, delete and submit interface. Since jobs and reservations will have a different workflow they can enjoy a different interface altogether.
I initially had it as only “periodic” but changed it to “server_periodic” after seeing how we name it in mom. I’ll change it back to periodic.
If the failover works as it is expected to work, I don’t see any problem with the secondary server executing the periodic hook. But for an active-active setup I think we can address that part of the issue only when we have active-active configuration. In case of active-active setup, periodic hooks may not undergo some special change that is not needed in other server hooks.
Speaking of failover, you might want to update interface 2 to what happens in a failover situation. Does the hook frequency start counting when the secondary takes over? Does it start counting when the secondary comes up, but the hooks don’t fire until after the secondary takes over? You should probably be specific.
IMHO, in interface 3 you probably don’t need the 2nd and 3rd message. The 4th message covers both of them. If you want to keep the two messages, I’d change the 4th message to include the default frequency. This way the user isn’t confused when they try and unset the frequency/alarm and the request is get rejected. They won’t know what the default is without looking it up.
Another option is if the user unsets one of frequency/alarm and would violate the 5s rule, you modify the other. So if the alarm was 5m and the user unsets frequency, you could either unset the alarm back to 30s or set it to 115s (default 120-5). I don’t know how much I like this option. This would be modifying something the admin specifically set.
Bhroam
It only happens when secondary takes over and gains control. I’ve added that bullet in interface 2. Thanks for catching this!
I think I’ll just do away with message 2 & 3. Last message covers the essence of what we wish to convey and can be used in all the places. Like you said, I’d also refrain from changing alarm/freq set by admin.
I’ve updated the document again.
Jon:
I would think that it would be better to execute the pbs.server().add() and remove() call when it is called vs when the hook is accepted. This allows for the hook writer to know if it succeeds or fails. If the hook writer calls a pbs.event().reject() then they are responsible to back out the changes before they reject the hook. I believe this is done in other places in the hook infrastructure but Al would know for sure.
[Al] This doesn’t fit in the current hook infrastructure design. Only when the hook ends with an accept() call (or implicit if no reject()) will things change in pbs such as job changes, new nodes and so on.
After some thought I’d like to see a change in how the frequency is handled. Right now we’re forcing the alarm to be shorter than the frequency. This creates a situation where you will never have a hook event fire when another one is running. What this doesn’t allow for is someone to have a short frequency and a long alarm.
What I’d rather see is that the server restarts the frequency every time it comes up. It will only fire off a hook event if there is not already one running. An example: We have a hook that will take 7m to run with a 5m hook frequency. After 5m into the hook run, the frequency comes up. The server sees there is a hook running does not start another hook. It only restarts the timer. The hook finishes 2m later. After a total of 10m the frequency comes up again and a new hook is fired off.
This allows for hook processes to take a variable amount of time without disturbing the hook frequency. If a hook writer knows their hook normally takes 2m but can take up to 10m at times. They don’t want their hook to be killed in the cases it does take 10m. The new solution I’m proposing will allow them to keep the frequency low and still allow for the occasional long run.
One last thing to think about: What happens to currently running hooks at the switchover during failover. What will happen in the case when the secondary is handing over control to the primary but has a running periodic hook? Do we let the hook finish even though the secondary is now idle? Probably not because if a hook is accepted, it needs to modify the database. If we’ve handed over control to the primary, we no longer can modify the database. Do we arbitrarily kill it off? Do we not hand over control until all the hooks have finished?
I think the cleanest option is to kill the running hooks.
That seems reasonable. I just want to make sure that the hook always start some n*interval for diagnostic purposes.
As for what happens to the current running hooks on failover, I don’t see a need to define this since if the server goes down (crashes) the hook will not be able to communicate with it and if the machine goes down then the hook will be terminated as well.
I was actually talking more on the side when the secondary goes idle and transfers control back to the primary server. The secondary process is still there. It’s just in an idle state. The hook is running in a separate process, so it’s still alive and well not knowing what happened to the secondary.
I’m OK with not defining this though. It’s a periodic hook. The primary will start a new one shortly.
Bhroam
Thanks for providing your comments!
I’ve updated the document again. Changes are made to Interface 3 and 2 new interfaces (12 and 13) are added.
Please have a look.
A couple of thoughts on the new interfaces:
Will submitting a job via a hook cause the queue job hooks to run (same with reservations)? I’m assuming yes.
In interface 13, you need to create your reservation before you set its start time.
Should you be able to associate a queue in the same way as a reservation? Something like j1.queue = q1 ?
You may want to add arguments to your executable attribute. Running ping with no arguments just prints its usage.
I like how the design is shaping up.
You are right about queue job hook run. Server will run this hook upon job submission.
About the queue association, I think the functionality can be extended to accept queue also but I don’t see a need for it. If a queue is already existing then one can query that queue and use it’s name and if a hook writer is creating a queue then the name of the queue is a mandatory attribute to create it. So, in both the cases queue name is already known to the hook writer.
I’ve made changes to the executable name and reservation submission.
Two questions.
On interface 4 the the reject message says “run periodic_hook; periodic request …” Should it just say “periodic_hook; periodic request …”
On interface 12 we have j2 = pbs.server().job(“12.host1”). Is job 12 already running and we are just making job 3 dependent on it?
Thanks for reviewing Jon!
In Interface 4 - “run_periodic_hook” is actually the function name that logs this message. It can not be changed to “periodic_hook” unless we change the function name itself.
Interface 12 - You are right, Job 12.host1 already exists and we are submitting another job which will be dependent on it.