parallel-consumer icon indicating copy to clipboard operation
parallel-consumer copied to clipboard

Missing feature to terminate processing

Open Zordid opened this issue 1 year ago • 6 comments

I just evaluated this library for use in our product. But sadly, the missing termination feature from within the poll lambda is a show stopper. How come such a trivial and highly useful feature is missing? If I can only fail a record over and over again, there must be a way to stop processing gracefully altogether. Is this feature coming soon?

And why is the poll lambda relying on exception handling instead of a proper return value? Using exception handling for flow control is very outdated and should not be the norm anymore.

Thanks!

Zordid avatar Feb 11 '24 18:02 Zordid

Hi @Zordid - can you please elaborate? I do not fully follow - what do you mean by termination feature?

Afaik you can close parallel consumer instance from within poll call - either signal main app thread that it should close the Parallel Consumer instance or just call the close from within poll directly on a different thread - something like this:

        parallelConsumer.poll(recordContexts -> {
            if (shouldSignalClose()) {
                CompletableFuture.runAsync(() -> parallelConsumer.closeDontDrainFirst(Duration.ofSeconds(10)));
            }
            processRecord(recordContexts);
        });

Normally though application would not close Parallel Consumer due to failing record - if the issue that leads to record processing to fail - is bad data in the record - then Dead Letter Queue approach could be used or that record just skipped and processing moved to next record - depending on requirements, but the general goal is to not block processing.

If the processing is failing due to external system being unavailable - then that could be monitored through health monitoring of dependent systems and ParallelConsumer can be paused / closed on unavailability of external system (similar to circuit breaker pattern) and resumed / restarted when health of external system is back to good / green - that, in my opinion, would be triggered outside of the Poll lambda though.

Using exceptions to indicate a failure is normal enough in applications to my knowledge - could you elaborate ?

rkolesnev avatar Feb 15 '24 10:02 rkolesnev

All I mean is, it is a cleaner approach to give the lambda a way to indicate process result by a return value than to use exceptions. General rule of thumb for me, because exceptions ar least make it very hard to understand the contract by looking at the signature of a function.

If the lambda that processes the record would need to return a plain enum with ok, retry or stop it would be much easier to understand.

Well, the reason stop is a thing we need is quite simple. The product we build does not know anything about its context, it is all driven by configuration of a customer.

And one of the possible reactions for errors is to shut down the application - the host environment would then restart it all and there is a chance the problem is solved.

If it were just a pure Kafka app, I'd agree, but we run completely different things connecting to the very unpleasant SAP world in this app...

Cheers and thanks for one trick to kill the processing from inside!

Zordid avatar Feb 15 '24 15:02 Zordid

How come such a trivial and highly useful feature is missing?

This is not the right attitude, specially for free software

krvajal avatar Mar 01 '24 06:03 krvajal

Using exceptions to indicate a failure is normal enough in applications to my knowledge - could you elaborate ?

I think I forgot to elaborate on this. IMHO exception have had their time. I am coming from a functional standpoint and love interfaces that are clear&sleek and use a functional approach rather than relying on (hidden) contracts of how exceptions are meant to be thrown and caught elsewhere. Exceptions are the modern goto, after all. Whenever possible, an API should try to be free of such inner behaviour and relying on user code to throw exceptions to trigger different paths of execution.

Zordid avatar Mar 02 '24 12:03 Zordid

Personally, in general - i have a view that "it depends" - how application / library is written, what expectations are of its users etc. I find that Java functional interfaces are a bit lacking when it comes to handling branching logic - pattern matching, exception flows etc - i've seen and used that more in Scala, or using something like Vert.x with future handler composition.

Specifically with Parallel Consumer - its code base is not written in functional style, and it does need to handle unhandled exceptions from user code with some default behaviour, and i would argue it would make sense to tie specific behaviour to exception types. That is widely used and familiar pattern to many Java developers. In addition - User Function already has a return value - for poll and produce flow. Of course it can be wrapped into Result object with status of execution and any data returned, but it just makes it more complex.

So - while i agree that there are cases when it is better and more natural to use return object with different statuses for success / failure modes - i do not think that is a good fit for Parallel Consumer code base.

rkolesnev avatar Mar 04 '24 09:03 rkolesnev

Keeping it open - as i believe it would be beneficial to implement - bandwidth permitting. Related enhancement that was closed as stale - but may have useful work in it already done / partially done - #291

rkolesnev avatar Mar 05 '24 09:03 rkolesnev