I noticed the EventFlusher interface extends the CommandContextCloseListener and believe it may have the same issue that was recently fixed for the AsyncJobAddedNotification class via commit (37e54d1006174416182f7be34f7fd1fd76233fce). Should the EventFlusher interface instead implement TransactionListener to ensure events are processed based on the spring transaction demarcation when using the SpringProcessEngineConfiguration? Assuming this is correct, I can submit a pull request for this change.
We did a good analysis when we did the AsyncJobAddedNotification change and the EventFlusher should not be changed based on our analysis. The AsyncAddedNotification should only be executed when the transaction is fully committed. So in case of a Spring transaction you only want to execute the job when the full transaction (including the job) has been persisted.
For the default DatabaseEventFlusher you would like to participate in the same transaction as the Flowable Engine persistence logic. So it can be committed when the command context is closed.
Let me know if you come to another conclusion.
I think the required behavior of the EventFlusher depends on the particular use case.
When the EventFlusher is being used as an audit trail as described in the documentation then I agree you want to participate in the same transaction as the engine persistence logic.
On the other hand if the EventFlusher is being used to send events which are used to track metrics, such as the average time spent in various tasks (i.e activities) across multiple instances of a process, then maybe guaranteed delivery isn’t necessary. In this case you may want to send the events only if the transaction completes successfully.
The DatabaseEventFlusher class seems to blur the lines a bit because it currently inserts the event data into the same database as the flowable tables and uses the CommandContext/DbSqlSession to do so which results in it having to implement CommandContextCloseListener. In an alternative EventFlusher implementation that sends events to a message system such as RabbitMQ or Kafka the use of the CommandContext/DbSqlSession wouldn’t be necessary.
In any case maybe all that’s needed is another Abstract Flusher that transfers the generated events from the command context to a TransactionListener to handle the second use case.