I've added a test for the `shutdownNode` method in a separate PR on your repository: let me know if it mirrors your expectation of what the method is supposed to do :+1:
Re performance implications of this change: I've benchmarked it and the `synchronized` block makes a pure in-memory execution (i.e. no I/O involved) of the ProcessorBase about 7 time slower on my box when biased locking is disabled to mimic predicatable latencies and to avoid the JVM to trick us (`-XX:-UseBiasedLocking` is explained [here](https://mechanical-sympathy.blogspot.it/2011/11/biased-locking-osr-and-benchmarking-fun.html)).
I have to do benchmarking on the overall broker too, but I suppose it will impact latencies if not the thoughput too.
IMHO would be better to implement this using only (collaborative) lock-free algorithms that allow predicatable (and lower) latencies on hot paths: I've pushed another PR with an implementation of it for our use case (to review/discuss).
On the hot paths it cost just a volatile load (on x86 is just a load and compare).
@franz1981 I'm surprised the lock would be that much slower.. there is nothing competing for that long..
The advantage of the lock here is reentrancy. it's now possible to shutdown within the current executor. (even though I won't recommend that).
I've tried a lock free thing using the state.. but it seemed simpler...
I will try your PR.. just one thing.. you sent the PR to a wrong branch.. should be clebertsuconic:artemis-1495
@clebertsuconic Argh!! Sorry about the wrong PR: I will repush it!
Just a couple of observations:
to be 100% fair, the lock solution is not always that slow (like the in memory case is showing) when I/O (networking) is involved: in that case is just ~15% slower (eg that's on my notebook that has a single socket, with servers would be greater due to the context switch & kernel space journey for each task).
> The advantage of the lock here is reentrancy.
:100: true! But I've avoided to implement it with a reentrant solution due to the advices received on a past PR: probably it is an antipattern (I suppose). If I'm wrong on that I can write a reentrant version too, but it will change further the logic (it is already complex!).
The solution with the lock is much simpler hence I understand 100% if the lock free version wouldn't be accepted and if you'll find any error on the logic of what I've done we can fix it together (before any merge) :)
Probably it could be made simpler too
@clebertsuconic I've just pushed a PR to your version with very few changes (but sadly seems more using diff on the code) mostly perf dependent and to make the code a lil more documented: I'm happy with your changes and the added test too, very well done!!!!! :+1:
Let me know if the changes I've pushed seems good considering what are your expectations on the mechanics of the final version.
Just a note (if possible): now we have some sanity tests on the OrderedExecutor hence I will be super happy to see the missing ones for the other critical methods, if any (eg: flush, shutdown)..if is not possible we'll add them later.