A design proposal for adding additional hook events for tracking reservation changes and state updates has been added. This would add a RESV_CONFIRM, RESV_BEGIN, and MODIFYRESV hook. This is to enable both better near time to real time tracking of reservations for custom site allocation charging and accounting purposes and to capture changes that would occur during the life of a reservation. Due to how closely related these three hooks are, it was thought that putting them together would ease the reviewing process.
A branch with the proposed changes has also been included. If things look good then I will put together the final pull request.
Hey @pmrich,
Thank you so much for designing these three hook events. They will help PBS greatly.
Here are some comments on your design document:
Could you add if the resv_begin hook runs before or after the scheduling cycle is kicked. I guess it doesn’t really matter because it is a read only hook, but it would be nice to know
For the resv_confirm event, it might be nice to allow the hook writer know why the reservation was being reconfirmed (modify, in-conflict, or degraded). Something to consider.
Modifying a reservation is different than submitting one. When you submit a reservation, there is no previous state, so all you care about in the hook is the current state of the reservation. When modifying a reservation, you will care about the previous state and the current state. This will give you the full picture of what it is changing from and what it is changing too.
In the internals section, the first bullet of resv_confirm mentions resv_begin. I think this is a copy/paste error
nit: there’s a typo in the 4th bullet of modify resv internal section. It says “mofifyresv”
Thank you for the comments. Bullets 4 and 5 have been fixed.
I have a question on the first bullet: From the code I read in the scheduler, it’s possible for the reservation to begin in the scheduler regardless of it’s status in the server, if I read that right. For example, the cycle begins a few seconds before the reservation begins and goes active, but during the cycle, it gets to the check to see if an advance reservation should be active and then activates it for the remainder of the cycle. This hook would be run after the cycle is started right after the B-accounting record is written. I’ve added a note about exactly when this would be run.
I’m checking into the second and third bullet now and will be addressing those after I look into them more. In the attached branch, I did add a resv_o data structure, I’ll make sure that this is doing what I think it is doing and document it. I modeled this off of the modifyjob hook.
So there are two halves of a reservation. The reservation itself and the reservation queue. These both make up the reservation. The internals of PBS uses the fact that the reservation queue is stopped up until the reservation starts. So it doesn’t actually matter if the reservation time passes while we’re in cycle. Unless the queue is started, we won’t run anything i it. The queue is started at the start of the reservation.
You’ll see this in the comment of jobs in the reservation before it starts. The comment will say the queue is not started.
For your third bullet, information on resv_o has been added to the documentation for
MODIFYRESV. It is currently in the branch, and works similarly to the way job_o works
for MODIFYJOB.
Finally, for your second bullet about including the reason for the confirmation or
reconfirmation in RESV_CONFIRM, I think this is a good idea but I would like to
defer implementation to a later pull request. In the branch so far, I’ve been
trying to keep this close to the other existing reporting and logging information as
I can and this is happening after the confirmation is complete and the reservation’s
state is updated. For logging and accounting purposes on reservations, this hook
currently is useful. Additionally, we are planning on doing a pass on the data
included in the reservation hooks and will likely be adding other information
in the near future. I think this would be a great candidate to add at that point.
I think this should address the remaining points you raised. Does this sound
reasonable to you and do you need further information or updates from me
on the design document?
If not, I’ll go ahead and generate the pull request.