D22446: Adding error handling strategy control to Scheduler

[Wolfgang Reissenberger]

wreissenberger planned changes to this revision.
wreissenberger added a comment.


  Many thanks for your comments. I will think about them and come back.

INLINE COMMENTS

> TallFurryMan wrote in scheduler.cpp:1431
> Here I tend to disagree. We have JOB_INVALID, JOB_ERROR and JOB_ABORTED.
> JOB_INVALID indicates a configuration error, and we cannot proceed with the job at all.
> JOB_ERROR indicates a fatal error during processing, and we cannot proceed with the job anymore.
> JOB_ABORTED indicates a transitory error during processing, and we should retry at some point.
> Based on these definitions, we should not re-evaluate JOB_ERROR.
> Now, I know that the enums are not properly ordered, but to me JOB_ERROR jobs should still be removed.

I would agree if JOB_ERROR is only used when an error is non recoverable. But for example what error state should be set if a slew command fails? It is definitely recoverable, since sending the command again solves the problem. The same is true if e.g. we loose network connection.

The problem is, that we do not have a JOB_FATAL state. For those, I absolutely agree, we should not try to restart. That's why I added the option to handle errors like aborts. I am quite sure that there are setups where restarting of errors is dangerous. But with my setting, its absolutely fine.

> TallFurryMan wrote in scheduler.cpp:1579
> I'm not sure about this block, it really gets in the way of the regular method of scheduling.
> It will conflict with the completion time of the aborted jobs.
> It will run a parallel conflict with whatever restriction makes the jobs aborted at some point.
> It will conflict with the preemptive shutdown option (unless you forbid a larger delay?).
> Eventually, forbid a delay larger than the lead time option?

This section simply extends the old predicate based to a loop so that it can take into account whether the "re-schedule errors" checkbox is selected. I needed to switch to a loop since the checkbox is not static.

> TallFurryMan wrote in scheduler.cpp:1602
> OK, only if scheduling algorithm takes the presence of JOB_ABORTED into account.

Sorry, don't get the point.

> TallFurryMan wrote in scheduler.cpp:3165
> This is a behavior change, but well, makes sense now.
> Note that currently, the completion date embeds both date and time.
> The completion date should be optional, so that multi-day schedules could be more easily programmed (I have a need for this).
> The interface is painful when rescheduling START_AT and COMPLETE_AT jobs, but not moving jobs should improve that.

Oops, multi-day schedules? That's tricky.

> TallFurryMan wrote in scheduler.cpp:3191
> I disagree: this job must be rescheduled to next observation time, thus JOB_ABORTED.

Maybe neither COMPLETE nor ABORTED. What about if we set it to EVALUATION?

> TallFurryMan wrote in scheduler.cpp:3217
> I disagree: this job must be rescheduled to next observation time (albeit quite a far one!), thus JOB_ABORTED.

see above on line 3191.

REPOSITORY
  R321 KStars

REVISION DETAIL
  https://phabricator.kde.org/D22446

To: wreissenberger, mutlaqja, TallFurryMan
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190714/7d67221d/attachment-0001.html>