From 72d9ef9ad4ca70752708068440305200c740dc7a Mon Sep 17 00:00:00 2001 From: jbirddog <100367399+jbirddog@users.noreply.github.com> Date: Wed, 15 Mar 2023 10:52:06 -0400 Subject: [PATCH 1/3] Provide more details in process instance locking errors (#181) --- .../routes/process_instances_controller.py | 18 ++++++++++++++++-- .../services/process_instance_queue_service.py | 15 ++++++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py index 252b9264..ec3015dd 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py @@ -55,9 +55,18 @@ from spiffworkflow_backend.services.error_handling_service import ErrorHandlingS from spiffworkflow_backend.services.git_service import GitCommandError from spiffworkflow_backend.services.git_service import GitService from spiffworkflow_backend.services.message_service import MessageService +from spiffworkflow_backend.services.process_instance_lock_service import ( + ProcessInstanceLockService, +) from spiffworkflow_backend.services.process_instance_processor import ( ProcessInstanceProcessor, ) +from spiffworkflow_backend.services.process_instance_queue_service import ( + ProcessInstanceIsAlreadyLockedError, +) +from spiffworkflow_backend.services.process_instance_queue_service import ( + ProcessInstanceIsNotEnqueuedError, +) from spiffworkflow_backend.services.process_instance_queue_service import ( ProcessInstanceQueueService, ) @@ -129,7 +138,11 @@ def process_instance_run( try: processor.lock_process_instance("Web") processor.do_engine_steps(save=True) - except ApiError as e: + except ( + ApiError, + ProcessInstanceIsNotEnqueuedError, + ProcessInstanceIsAlreadyLockedError, + ) as e: ErrorHandlingService().handle_error(processor, e) raise e except Exception as e: @@ -143,7 +156,8 @@ def process_instance_run( task=task, ) from e finally: - processor.unlock_process_instance("Web") + if ProcessInstanceLockService.has_lock(process_instance.id): + processor.unlock_process_instance("Web") if not current_app.config["SPIFFWORKFLOW_BACKEND_RUN_BACKGROUND_SCHEDULER"]: MessageService.correlate_all_message_instances() diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_queue_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_queue_service.py index d9f900b2..d75d903f 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_queue_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_queue_service.py @@ -14,6 +14,10 @@ from spiffworkflow_backend.services.process_instance_lock_service import ( ) +class ProcessInstanceIsNotEnqueuedError(Exception): + pass + + class ProcessInstanceIsAlreadyLockedError(Exception): pass @@ -62,15 +66,20 @@ class ProcessInstanceQueueService: db.session.query(ProcessInstanceQueueModel) .filter( ProcessInstanceQueueModel.process_instance_id == process_instance.id, - ProcessInstanceQueueModel.locked_by == locked_by, ) .first() ) if queue_entry is None: + raise ProcessInstanceIsNotEnqueuedError( + f"{locked_by} cannot lock process instance {process_instance.id}. It" + " has not been enqueued." + ) + + if queue_entry.locked_by != locked_by: raise ProcessInstanceIsAlreadyLockedError( - f"Cannot lock process instance {process_instance.id}. " - "It has already been locked or has not been enqueued." + f"{locked_by} cannot lock process instance {process_instance.id}. " + f"It has already been locked by {queue_entry.locked_by}." ) ProcessInstanceLockService.lock(process_instance.id, queue_entry) From efcc083638345a59b730e695f431710c42edf03a Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 15 Mar 2023 12:26:47 -0400 Subject: [PATCH 2/3] Columns should not be removed on reset, but any filters applied to those columns should be removed. --- .../src/components/ProcessInstanceListTable.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx b/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx index 9f8d258c..04710605 100644 --- a/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx +++ b/spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx @@ -704,11 +704,8 @@ export default function ProcessInstanceListTable({ setEndToTime(''); setProcessInitiatorSelection(null); setProcessInitiatorText(''); - if (reportMetadata) { - reportMetadata.columns = reportMetadata.columns.filter( - (column) => !column.filterable - ); + reportMetadata.filter_by = []; } }; From 12f0dc5315aeba214a5f7f3857db5a2c1ce46aec Mon Sep 17 00:00:00 2001 From: jbirddog <100367399+jbirddog@users.noreply.github.com> Date: Wed, 15 Mar 2023 12:32:55 -0400 Subject: [PATCH 3/3] Smaller locking window for the background processor (#183) --- .../process_instance_queue_service.py | 37 ++++++++++++++----- .../services/process_instance_service.py | 6 +-- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_queue_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_queue_service.py index d75d903f..a0aceb94 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_queue_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_queue_service.py @@ -1,5 +1,6 @@ import time from typing import List +from typing import Optional from flask import current_app @@ -84,8 +85,33 @@ class ProcessInstanceQueueService: ProcessInstanceLockService.lock(process_instance.id, queue_entry) - @staticmethod + @classmethod + def entries_with_status( + cls, + status_value: str = ProcessInstanceStatus.waiting.value, + locked_by: Optional[str] = None, + ) -> List[ProcessInstanceQueueModel]: + return ( + db.session.query(ProcessInstanceQueueModel) + .filter( + ProcessInstanceQueueModel.status == status_value, + ProcessInstanceQueueModel.locked_by == locked_by, + ) + .all() + ) + + @classmethod + def peek_many( + cls, + status_value: str = ProcessInstanceStatus.waiting.value, + ) -> List[int]: + queue_entries = cls.entries_with_status(status_value, None) + ids_with_status = [entry.process_instance_id for entry in queue_entries] + return ids_with_status + + @classmethod def dequeue_many( + cls, status_value: str = ProcessInstanceStatus.waiting.value, ) -> List[int]: locked_by = ProcessInstanceLockService.locked_by() @@ -102,14 +128,7 @@ class ProcessInstanceQueueService: db.session.commit() - queue_entries = ( - db.session.query(ProcessInstanceQueueModel) - .filter( - ProcessInstanceQueueModel.status == status_value, - ProcessInstanceQueueModel.locked_by == locked_by, - ) - .all() - ) + queue_entries = cls.entries_with_status(status_value, locked_by) locked_ids = ProcessInstanceLockService.lock_many(queue_entries) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py index dfeb2bde..23ce9a22 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py @@ -84,15 +84,15 @@ class ProcessInstanceService: @staticmethod def do_waiting(status_value: str = ProcessInstanceStatus.waiting.value) -> None: """Do_waiting.""" - locked_process_instance_ids = ProcessInstanceQueueService.dequeue_many( + process_instance_ids_to_check = ProcessInstanceQueueService.peek_many( status_value ) - if len(locked_process_instance_ids) == 0: + if len(process_instance_ids_to_check) == 0: return records = ( db.session.query(ProcessInstanceModel) - .filter(ProcessInstanceModel.id.in_(locked_process_instance_ids)) # type: ignore + .filter(ProcessInstanceModel.id.in_(process_instance_ids_to_check)) # type: ignore .all() ) process_instance_lock_prefix = "Background"