Hi Rob,
Great to see these changes. I am a little confused about this change in
Queue.java:
+ if (list != null) {
+ synchronized (consumers) {
+ for (MessageReference node : list) {
+ Subscription target = null;
+ List<Subscription> targets = null;
+ for (Subscription s : consumers) {
+ if (dispatchSelector.canSelect(s, node)) {
+ if (!s.isFull()) {
+ s.add(node);
+ target = s;
+ break;
+ } else {
+ if (targets == null) {
+ targets = new ArrayList<Subscription>();
+ }
+ targets.add(s);
+ }
+ }
+ }
+ if (targets != null) {
Shouldn't this if statement be?
if (target == null && targets != null)
I would have thought if you already found a matching subscriber that
isn't full, then there is no need to check the list of other full
subscribers that were found?
+ // pick the least loaded to add the messag too
+
+ for (Subscription s : targets) {
+ if (target == null
+ || target.getInFlightUsage() > s
+ .getInFlightUsage()) {
+ target = s;
+ }
+ }
+ if (target != null) {
+ target.add(node);
+ }
+ }
If what I said above is true, then the immediately above if statement
needs to be moved outside its enclosing if - otherwise it only gets
executed when targets != null. We'd want this to execute if we found a
matching target wouldn't we?
--
Cheers,
David
Nuix Pty Ltd
Suite 79, 89 Jones St, Ultimo NSW 2007, Australia Ph: +61 2 9280 0699
Web: http://www.nuix.com Fax: +61 2 9212 6902
