-
Notifications
You must be signed in to change notification settings - Fork 355
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
Stop filling monitoring queues when processor fails #4697
Conversation
b2aec63
to
e5e32bf
Compare
// Send some requests to process some statistics. | ||
request(ERRORS_BEFORE_FAIL); | ||
// Give some time to the scheduler to collect data. | ||
Thread.sleep(TIMEOUT); |
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 CountdownLatch be used instead of sleep? Sleep tends to create intermittent failures.
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.
In the first Thread.sleep we can do it because it is about waiting till something happens.
But in the second we cannot do it, because nothing should happen. So we need to wait explicitly and check.
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 was trying to add a CountDownLatch for the first one but it is not easy. There are 2 ways:
Easy one. It is about checking the queue till the size is 0:
private void waitTillQueueIsEmpty() throws InterruptedException {
final CountDownLatch latch = new CountDownLatch(1);
ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor();
scheduler.scheduleAtFixedRate(() -> {
Response response = target("/example/queueSize").request().get();
int queueSize = response.readEntity(Integer.class);
if (queueSize == 0) {
latch.countDown();
}
}, 0, 10, TimeUnit.MILLISECONDS);
boolean wait = latch.await(TIMEOUT, TimeUnit.MILLISECONDS);
scheduler.shutdown();
assertTrue("The queue was not empty in the given time", wait);
}
But this doesn't work always, because it could happen the scheduler consumed all the events and the size is 0, but new events are coming.
The difficult option:
Pass the CountDownLatch reference to the Runnable in the Scheduler of MonitoringStatisticsProcessor, so it will count it down every time an event is consumed. This is tricky because the way it is done it is not possible. I would need to make some changes in MonitoringStatisticsProcessor to achieve this.
I think it is not necessary to over complicate it, because the scheduler is running every 1 millisecond and it is very unlikely that during 1 second it was not able to do the work. But let me know what you think.
ebe41ef
to
540775d
Compare
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Related to this #4152 but there are more things to investigate.
This PR is about not produce new events in the queues when there is no consumer.