Reset pending gauge on handler unregistered #98
Conversation
Mirroring behaviour in HandlerRegistration Fixes vert-x3#95
|
Now I don't know if this needs to be cherry-picked to master ... as it seems in master vertx-core we are not discarding the pending events on unregister? (or maybe it's still done but in a different place, I couldn't find that) |
There was a problem hiding this comment.
Can you add a test for this @jotak ? I believe this is the right thing to do.
|
@jotak would you mind to check if there's a similar issue with the Dropwizard implementation? Thank you |
|
@tsegismont About adding a test: I struggle to make something reproducible about pending messages. I can easily check that after unregistration there's no pending message, but that wouldn't prove anything about what we do here. Do you have any suggestion? |
|
Le mer. 19 févr. 2020 à 10:19, Joel Takvorian <notifications@github.com> a
écrit :
@tsegismont <https://github.com/tsegismont>
About dropwizard, I believe there's nothing to do: see
https://github.com/vert-x3/vertx-dropwizard-metrics/blob/master/src/main/java/io/vertx/ext/dropwizard/impl/EventBusMetricsImpl.java#L251-L253
Pending gauges are already updated on unregistering.
👍
About adding a test: I struggle to make something reproducible about
pending messages. I can easily check that after registration there's no
pending message, but that wouldn't prove anything about what we do here. Do
you have any suggestion?
Create a worker verticle that registers a consumer.
Send a few messages from the test body.
In the consumer body, block the worker thread after the first message is
received
While the worker thread is blocked you can verify there are a few pending
messages.
After the worker thread is unblocked unregister the consumer.
I believe this should do
|
| if (first.getAndSet(false)) { | ||
| step2FirstReceived.complete(); | ||
| } | ||
| step3BlockWorker.await(); |
There was a problem hiding this comment.
@tsegismont I did more or less as you suggested, with a difference: event consuming is blocked on the event loop and not on worker thread. I know it can be a bit risky on a slower environment - here it works on my machine (let's see what CI says).
I tried with a worker thread as well, using:
WorkerExecutor executor = vertx.createSharedWorkerExecutor("my-worker-pool", 1);
// then in consumer handler:
executor.executeBlocking(promise -> { /*block*/... });
But it doesn't work, as this is not blocking thequeue processing (pending count is decreased despite blocking, here).
Or is there another way to do, such as telling vertx to use my worker for EB queue processing?
There was a problem hiding this comment.
Actually, I think I understand how to do: now using .setWorker(true).setWorkerPoolSize(1) on my deployment options... is that what you expected?
There was a problem hiding this comment.
It doesn't work without .setWorkerPoolSize(1) ?
There was a problem hiding this comment.
It does ... i thought it would be necessary to have the other events blocked, but it's not. I removed it.
|
thanks @tsegismont , i'll cherry-pick to 3.9 and 4 |
Mirroring behaviour in HandlerRegistration
Fixes #95