-
Notifications
You must be signed in to change notification settings - Fork 473
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
Fix testing queue/queue-rabbitmq - RabbitMQMailQueueTest.deQueueMails - RabbitFluxException #1075
Conversation
… - RabbitFluxException
...ueue/queue-rabbitmq/src/test/java/org/apache/james/queue/rabbitmq/RabbitMQMailQueueTest.java
Show resolved
Hide resolved
…meoutException - Regression: org.apache.james.queue.rabbitmq.RabbitMQMailQueueTest$MailQueueSizeMetricsEnabled.dequeueShouldBeConcurrent (from org.apache.james.queue.rabbitmq.RabbitMQMailQueueTest) Condition with lambda expression in org.apache.james.queue.api.MailQueueContract that uses java.util.concurrent.ConcurrentLinkedDeque, java.util.concurrent.ConcurrentLinkedDequeint was not fulfilled within 1 minutes.
}).subscribeOn(Schedulers.elastic())) | ||
}).subscribeOn(Schedulers.newBoundedElastic(2, 20, "DeQueueSchedulerTest"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failing test percent (Mean of 3)
Before: 7 %
After: 2.333 %
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better if we can understand the root cause IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried single scheduler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried single scheduler?
- Schedulers.single() -> .block is not allowed on single scheduler.
- reduce newBoundedElastic thread size to 1 -> the failing probability seems reduce a bit. about <= 2%
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2
will be easier to detect the parallel vulnerable. Furthermore here is the test code, IMO, no need to be too strict 1 or 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some tests for easier debugging issues with reactor-rabbitmq (maybe?)
See d40cdea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you see in these tests if using boundedElastic/single helps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open a ticket upstream too maybe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open a ticket upstream too maybe.
Where I can? https://issues.apache.org/jira/projects/JAMES ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you see in these tests if using boundedElastic/single helps?
single
: No, it fails as immediatelyboundedElastic
: it makes the % fail decreasingly
Scheduler newBoundedElastic = Schedulers.newBoundedElastic(10 * Runtime.getRuntime().availableProcessors(), 10, "MailQueueTest"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not see written answers to quan questions.
I do not understand the rationnals behind this.
Maybe the test should just be disabled?
I'm not sure for threads deeper here, I guess the creation threads as "elastic" make test cases sometimes fail (rarely). |
I added some tests for easier debugging issues with reactor-rabbitmq (maybe?) |
My feeling is that this could explain crashing consumers cc @Arsnael |
The crashing consumers we have sometimes in prod? Could be yes... |
Does usenio in rabbitmq client setting affects this? |
See #1099 |
CI sometimes pass, sometimes fail at
org.apache.james.queue.rabbitmq.RabbitMQMailQueueTest
Error logs