Skip to content

fix: Order sqs messages#1181

Open
idhir-Cacib-armoniK wants to merge 7 commits intoaneoconsulting:mainfrom
CACIB-ArmoniK-Admin:im_cacib/fixOrderMessages
Open

fix: Order sqs messages#1181
idhir-Cacib-armoniK wants to merge 7 commits intoaneoconsulting:mainfrom
CACIB-ArmoniK-Admin:im_cacib/fixOrderMessages

Conversation

@idhir-Cacib-armoniK
Copy link
Copy Markdown
Contributor

@idhir-Cacib-armoniK idhir-Cacib-armoniK commented Mar 16, 2026

Motivation

The SQS messages pushed are ordered by the group and priority however it can not ensure that the highest priority will got first.

Description

  • Arrange the code to avoid unecessary OrderbyDesc.
  • Add the unit test proving that we need to review the code.
  • Performing the code to have an order descending by Priority when pushing messages.

Testing

A unit test was added to generate randome messages through priorities and ensure that first priority messages will be sent first.

Impact

Impact the messages with the highest priority will be taken first.
On SQS Consumers, the queue enlisted will be done from highest priority to the lowest.

Additional Information

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • I have thoroughly tested my modifications and added tests when necessary.
  • Tests pass locally and in the CI.
  • I have assessed the performance impact of my modifications.

Comment thread Common/src/ArmoniK.Core.Common.csproj Outdated
@idhir-Cacib-armoniK idhir-Cacib-armoniK force-pushed the im_cacib/fixOrderMessages branch from 73b0619 to f78c36d Compare March 18, 2026 14:10
Comment on lines -71 to 73
for (var i = 0; i < int.Max(options_.MaxPriority,
1); i++)
var maxPriority = int.Max(options_.MaxPriority,
1);
for (var i = maxPriority; i >= 1; i--)
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loop boundaries are not the same.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes and there was fat finger regarding the code below if you see. i +1 so we will begin from 11 to 2. I'll change the code below to have priority directly if ok for you :)

Comment thread Common/tests/TaskLifeCycleHelperTest.cs Outdated
@idhir-Cacib-armoniK
Copy link
Copy Markdown
Contributor Author

make it work first no need to run pipeline for nothing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rebase/squash so that this file is not touched in the history

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rebase/squash so that this file is not touched in the history

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

- Arrange the code to avoid unecessary OrderbyDesc.
- Add the unit test proving that we need to review the code.
- Performing the code to have an order descending by Priority when pushing messages.
- Arrange the code to avoid unecessary OrderbyDesc.
- Performing the code to have an order descending by Priority when pushing messages.
- Remove fat finguer.
- Arrange the code to avoid unecessary OrderbyDesc.
- Performing the code to have an order descending by Priority when pushing messages.
- Remove fat finguer.
- Arrange the code to avoid unecessary OrderbyDesc.
- Performing the code to have an order descending by Priority when pushing messages.
- Remove fat finguer.
- Arrange the code to avoid unecessary OrderbyDesc.
- Performing the code to have an order descending by Priority when pushing messages.
- Arrange the code to avoid unecessary OrderbyDesc.
- Performing the code to have an order descending by Priority when pushing messages.
- Style for .csproj and tests after removal
@idhir-Cacib-armoniK idhir-Cacib-armoniK force-pushed the im_cacib/fixOrderMessages branch from aa59b6a to 4817cae Compare March 27, 2026 09:06
- Arrange the code to avoid unecessary OrderbyDesc.
- Performing the code to have an order descending by Priority when pushing messages.
- Style for .csproj and tests after removal.
- Put test as before as no need tests for this one :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants