Hi David,
On 19 Feb 2008, at 05:09, David Sitsky wrote:
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)
Yes it should!- changing ...
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?
Don't think so? We only want the message going to one subscription? I
may have misunderstood what you mean!
cheers,
Rob
--
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
