-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize Workflow Execution and Messaging #5243
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The trigger indexing in the workflow populator is now conditional. A boolean parameter has been added to the PopulateStoreAsync and AddAsync methods to determine whether to index triggers or not. Additionally, some code cleanups and refactoring have been made for efficient and cleaner code.
The method `AddAsync` in `DefaultWorkflowRegistry` has been updated to include a new first parameter set to true. This change aligns with recent modifications to the `AddAsync` method signature, ensuring proper function execution.
The updated GitHub workflow now includes triggers for branches with 'feat/*', 'enh/*', 'perf/*', 'hotfix/*', and 'chore/*' prefixes. This is to ensure that the workflow runs not only for the main, feature, issue, bug, enhancement, patch, and fix branches, but also on all new branches, improving coverage and visibility on all changes.
This commit introduces a new method, FindByIdAsync, to the WorkflowInstanceManager service. This method fetches a WorkflowInstance using its Id. Also, an interface declaration for the new method is added to IWorkflowInstanceManager.
The code for creating workflow definition filters has been refactored for brevity. Additionally, two sets of overloaded methods named `PopulateStoreAsync` and `AddAsync` were added to "IWorkflowDefinitionStorePopulator" and implemented in "DefaultWorkflowDefinitionStorePopulator". These methods allow specifying whether triggers should be indexed.
The refactoring is focused on an improved way of finding and passing ActivityDescriptor within WorkflowDefinitionActivity class. Previously, the service provider was passed to the DeclareInputAsVariables and DeclareOutputAsVariables methods, leading to a less readable and harder to maintain code. Now, we pass the ActivityDescriptor directly, making the code easier to understand and modify.
Fixes have been applied to the PolymorphicObjectConverter by adding the handling of TargetException. Additionally, the System.Reflection namespace has been included, and the addSetMethod invocation for the HashSet has been streamlined for better readability and performance.
This change simply removes an unneeded line of whitespace in the corresponding Middleware file. This change is consistent with the goal of maintaining clean and easy-to-read code.
Systematic refactor of the MassTransitWorkflowDispatcher class which initially focused on restructuring the DispatchAsync methods. New methods have been added that deal specifically with triggering and bookmarking workflows thus enhancing the readability of the code while also improving its autonomous function. The logging for non-found workflows has been improved as well.
Evicting the cache prior to triggers being indexed fixes a bug where publishing workflow changes would not result in new triggers being found.
The commit adjusts calls to AddAsync in DefaultWorkflowRegistry and DispatchAsync in DefaultWorkflowInbox to improve readability. Also, it marks DispatchResumeWorkflows and DispatchTriggerWorkflows in the Elsa.MassTransit.Messages namespace as obsolete, indicating their pending removal in future releases.
raymonddenhaan
approved these changes
Apr 18, 2024
src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs
Show resolved
Hide resolved
src/modules/Elsa.MassTransit/Services/MassTransitWorkflowDispatcher.cs
Outdated
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverter.cs
Outdated
Show resolved
Hide resolved
raymonddenhaan
requested changes
Apr 18, 2024
src/modules/Elsa.MassTransit/Services/MassTransitWorkflowDispatcher.cs
Outdated
Show resolved
Hide resolved
The changes remove duplication and improve readability by extracting the code responsible for dispatching a workflow into a separate method called DispatchWorkflowAsync. This method creates a workflow instance, gets the send endpoint, and then sends the message.
This commit simplifies the two separate catch blocks for NotSupportedException and TargetException into a single block using the new 'or' pattern in C#. It also makes minor adjustments to improve the clarity and readability of the code relating to the 'addSetMethod' invocation.
…tcher.cs Co-authored-by: raymonddenhaan <[email protected]>
…/elsa-core into enhancement/optimizations
The MassTransitWorkflowDispatcher.cs file is updated to improve readability and clarity. This includes changing the way bookmark and trigger filter objects are initialized, by breaking down the single-line initialization into multiple lines. Additionally, some logic has been updated in the DispatchBookmarksAsync function for better handling of workflow instance properties and input merging.
raymonddenhaan
approved these changes
Apr 18, 2024
The SendHttpRequestBase activity in the Elsa.Http module is updated to utilize the ILogger service. This extension enables the capture of HttpRequestException and TaskCanceledException events and logs their warnings, providing insight into potential issues during HTTP request sending.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces several optimizations and improvements to enhance the performance and reliability of Elsa Workflows:
Optimized Workflow Dispatching: Modifies the workflow inbox service to first identify all relevant workflow instances before dispatching execution requests. This change ensures that "dispatch workflow" messages are only sent when applicable, significantly reducing unnecessary message traffic.
Enhanced Message Handling: Ensures workflow inputs are stored with the workflow instance prior to dispatching messages to the queue. This approach prevents potential message rejection due to input mismatches at the service bus level.
Robust JSON Serialization: Improves the custom JSON serializer to handle
TargetTypeExceptions
more effectively. This enhancement prevents errors during deserialization when the target type is not suitable.Streamlined Application Startup: Reduces redundancy in the workflow startup process by indexing workflow triggers only once after all workflows are populated, thereby improving the startup time.
These changes aim to streamline workflow processing, reduce overhead, and improve overall system stability.
Fixes #5241