ARTEMIS-1925 - allow redistribution with loadbalance type of OFF to e…

[gtully]

…nsure local consumers get priority, we only optionally redistribute when messages are stuck

[michaelandrepearce]

-1 This is semantic breaking change for an existing setting and behaviour, e.g. im using OFF to ensure redistribute never occurs, regardless. it should never redistribute.

What you're doing is almost making off have almost the same semantic of ON DEMAND.

If the current semantic isn't what you need, suggest introducing a new one, to avoid causing a break / semantic change on existing users.

[michaelandrepearce]

see comment

[gtully]

e.g. im using OFF to ensure redistribute never occurs, regardless. it should never redistribute.
in my reading of the code and doc, this is just a broken assumption.
disable redistribution by not enabling redistribution.

redistribution is a solution to the problem of stuck messages due to early load balancing at routing time.
what is nice is that it is separate and it can have a binary behaviour.
5.x has deceaseNetworkConsumerPriority that biases towards local consumers. enabling redistribution can make such a feature binary (and better) when combined with no loadbalancing on initial routing.

ON_DEMAND is like STRICT when there are remote consumers, if we only want to go remote when we have stuck messages, then redistribution is the nice answer.
this change makes that possible by extending what is there in a natural way.

the change in semantic, that needs to be communicated for sure, but it need not be a blocker.
the real semantics are in the code and test and git history, all the tests are fine with this change and the new test verifies it.

[michaelandrepearce]

I think it should be a blocker. Especially when theres an alternative option thats easily to implement and avoids a break change that is simply adding an additional config option for the new semantic/behaviour

[gtully]

wha do you suggest in terms of configuration? to my mind more configuration in this area is more confusion.

[gtully]

as config: redistribution-delay-when-load-balance-type-off with default -1, it just does not make sense. It is mixing two things. I just don't think the breaking case is worth protecting, just don't enable redistribution-delay if you don't want redistribution.
is there more to it than that?

[michaelandrepearce]

the issue is loadbalancing and redistribution was intertwined, e.g. most of the time the reason for loadbalancing was in cluster setup and in that scenario you wanted redistribution to occure, its why the other settings such as ON DEMAND also have an effect

[AntonRoskvist]

The same basic thing was attempted here:
#2893

I for one would really welcome this change, weather it's a separate option or the default behavior. I see a lot of excessive "forwarding" caused by ON_DEMAND when the feature I am really after is just the redistribution.

Br,
Anton

[gtully]

@michaelandrepearce re: "the issue is loadbalancing and redistribution was intertwined" is exactly the problem, they should be separate and I want to fix that. Not make it more complicated, simplify. They are already separately configured but intertwined in code. This is a perfect release note . If it is a problem for some existing broken config, then there is existing config that can sort the problem.

[michaelpearce-gain]

I understand you want to do something here, but i still hold strongly especially as no need to break existing setups, can start brining new config values in very safely and mark old as deprecated if eventually we want to retire something in a 3.x release, but this you can easily add your new logic onto that new config.

[gtully]

the "break existing setups" is actually "point out existing broken setups", and I think the existence of such configs is likely to be small. I say that b/c redistribution is enabled (non default) but won't be in play due to the load balance policy. With redistribution being configured, if it now kicks in it will be just a 'fix'.
I agree in general, but in this specific case, new config won't make thinks better. The existing config being mixed up is the problem.
the problem I have is what to use for new config names, it just does not make sense to me. What do you suggest?

[michaelpearce-gain]

OFF_DEMAND? It then follow the ON_DEMAND, where LB is on, and distribution is demand based.

or even more explicit what it is, could just be:

OFF_WITH_DISTRIBUTION

[gtully]

I really think this is a step in the wrong direction, but I will go with OFF_WITH_DISTRIBUTION, which make redistribution conditional on loadbalance type ON_DEMAND or OFF_WITH_DISTRIBUTION && redistribution-delay > 0

To my mind, load balancing happens at routing time, redistribution on a queue with stuck messages. They should be independent. Maybe for 3.0 we can resolve this :-)

[gtully]

There is a new load balance type - OFF_WITH_REDISTRIBUTION that ensures local consumers get priority, ie: no forwarding on initial routing, but the potential to redistribute if configured

[gtully]

@michaelandrepearce can you close the loop on this one, thanks!

[michaelandrepearce]

@gtully should be ok to merge now.