Bug in `analytics-sample`, `BlockingFlush`, "Attempt to register more than 65535 parties for java.util.concurrent.Phaser"

[CraigStuntzWillowTree]

In the analytics-sample code there is a BlockingFlush plugin which uses java.util.concurrent.Phaser incorrectly.

The problem is if you actually try to use this in a production scenario it calls phaser.register() lots of times, but it never calls phaser.arriveAndDeregister(). Instead it calls phaser.arrive(). So after some time you will receive:

java.lang.IllegalStateException: Attempt to register more than 65535 parties for java.util.concurrent.Phaser@6cb672ad[phase = 0 parties = 65535 arrived = 65534]

I can't see how the BlockingFlush can work at all as-is. I think it needs to call phaser.arriveAndDeregister() instead?

[pbassut]

Well, it's just a sample to show how the library should be used. We don't expect people to leave that running for a long time.

If you were to use that in a production environment, then you would probably have to adjust it accordingly.

But maybe it's cool to have a fully functional sample code.
PRs are welcome!

Good catch, btw.

[CraigStuntzWillowTree]

So with it changed to phaser.arriveAndDeregister() it seems to work correctly, although if another plugin throws an exception in either its callback or its messageTransformer method it will still fail. I don't consider it a robust solution, but something is clearly needed here as without this plugin there are orphaned messages left in the queue when I tear down Analytics. I think a robust solution probably involves changing analytics-java to expose the AnalyticsClient.messageQueue.isEmpty() method, so that a user of Analytics can wait until the queue is drained before destroying it.

[CraigStuntzWillowTree]

I am still having problems with too many registered parties to the phaser even after making this change. It happens less often, but it still happens. My guess is that transform and success/failure events are somehow not perfectly symmetrical. At any rate, I've replaced BlockingFlush with... nothing? And it seems to work. AnalyticsClient ensures that the queue is fully drained when you call shutdown() and that's all I really care about.

[pbassut]

Right. That's done by placing a STOP signal into the queue. So no shutdown happens until the runner gets to that STOP signal and starts the shutdown(by that point it already processed every message in the queue).

But glad you got it working the way you wanted!

[philgebhardt]

Anyone coming here looking for some more information, I've outlined what a change looks like, using arriveAndDeregister, along with a breakdown of how indefinite blocking can occur with the current example. #471