gruelbox / transaction-outbox

Reliable eventual consistency for Microservices
Apache License 2.0
209 stars 37 forks source link

Allow TransactionOutbox#scheduler ThreadFactory customization #688

Open reda-alaoui opened 1 month ago

badgerwithagun commented 3 weeks ago

I think this would be more neatly solved by as noted on #687 by allowing a ScheduledExecutorService to be supplied when building the TransactionOutbox. You should have the option to be entirely control of your executor lifetimes rather than expecting the outbox to do it.

reda-alaoui commented 3 weeks ago

@badgerwithagun , by default, TransactionOutbox#scheduler uses a single thread pool. I thought this was important to keep this number of threads to avoid potential concurrency issues. Letting applications pass a full executor instead of a ThreadFactory will allow the formers to provide multi threaded executors.

If you think there is no concurrency issue with an executor holding more than 1 thread, we could consider there is resource waste issue and/or an API surface that is too wide if 1 thread is strictly enough.

Let me know what you think of that :)

badgerwithagun commented 3 weeks ago

@reda-alaoui I'm fine with the JAvadoc just making it clear that the ScheduledExecutorService should only have one executing thread and queue concurrent requests. By default it will use the existing implementation which works fine. If a user starts messing around with it, they should know what they are doing!