O3-5765: Automatically clear queue entries on a schedule - #119
O3-5765: Automatically clear queue entries on a schedule#119UjjawalPrabhat wants to merge 9 commits into
Conversation
b9b4520 to
fb2a13d
Compare
dkayiwa
left a comment
dkayiwa
left a comment
There was a problem hiding this comment.
The PR description still describes an earlier version of this. It says the default is 23:59, warns that existing deployments will begin clearing all queues at end of day once deployed, and describes the task as acting once the clock passes the configured time. None of that holds any more: the property defaults to blank, and the task works back from the most recent occurrence of the configured time. Could you refresh it? It is the first thing anyone coming to this PR reads, and as written it advertises the behaviour that got changed.
There was a problem hiding this comment.
While I realize this wasn't added by this module, it would be better to use core's task scheduling features rather than ScheduledExecutorTask. That said, if we are going to use Spring scheduling, then do not call Context.getService() but inject the service from Spring.
Both tasks are now AbstractTasks registered as TaskDefinitions when the module starts, following the pattern the reference application and chartsearchai activators use. That retires QueueTaskExecutor and QueueTimerTask along with the daemon token plumbing, since core runs scheduled tasks as the daemon user itself, and replaces the two hand rolled re-entrancy flags with the isExecuting guard AbstractTask provides. Implementers get both tasks on the Manage Scheduler page, where the interval can be changed or a task stopped. Nothing here overrides that afterwards: the scheduler starts tasks with startOnStartup at server startup and restores them across a module being started or stopped, so registration only happens once. The definition is scheduled as it is created, which is the one case the scheduler does not cover, of this module being installed into a running server. Services stay behind Context lookups because a task built by TaskFactory is not a Spring bean, which is what core's own AutoCloseVisitsTask does.
@ibacher Tried out using core's scheduling feat....have a look!! |
The unit tests stub the services the task talks through, so nothing checked that the search criteria filter the way they assume, that the save survives validation, or that endedAt reaches the database. This builds the task the way the scheduler does, from the class name on a TaskDefinition, and reads the entry back from the database rather than from the session the task used.
|
@claude review |
NethmiRodrigo
left a comment
NethmiRodrigo
left a comment
There was a problem hiding this comment.
Thanks @UjjawalPrabhat! A few suggestions -
Close queue entries through a new QueueEntryService.closeQueueEntry, which reloads the entry and writes it with optimistic locking, so an entry transitioned between the query and its turn in the loop is left alone instead of being overwritten from a stale snapshot. Both tasks use it, and the unreachable closeActiveQueueEntries is removed. Also flush and clear the session every 250 entries so the first sweep after a close time is configured stays linear, wrap the whole task registration so a lookup failure cannot skip the second task, and tighten a couple of log messages.
|
5 participants
Summary
Adds
AutoCloseQueueEntryTask. Each minute it ends active entries whosestartedAtis on or before the most recent occurrence of the configured time of day. So a run missed at that time is caught up by the next one, and entries started since then stay in the queue.Both this and the existing visit-close task are now registered with core's scheduler as
TaskDefinitionsfromQueueModuleActivator, replacing the module's ownQueueTaskExecutorandQueueTimerTask. They appear on the Manage Scheduler page, where the interval can be changed or a task stopped.Two new global properties:
queue.autoCloseQueueEntriesAtTime-HH:mm, blank by default, which disables clearing. An outpatient clinic would set 23:59.queue.autoCloseQueueEntriesForQueues- comma-separated queue uuids; blank means all queues.Blank by default so nothing changes for an existing deployment until an implementer opts in (O3-2443).
Related Issue
O3-5765