Deployers Ordering Issue

[ropalka]

This is the follow up with user forum discussion and relates to JBDEPLOY-201.

[ropalka]

Sure, we're discussing it now, don't we?
Regarding my commits, I can create separate branch from trunk if you have problem with it. But commiting in iterations in usually the way how I work :(

"Ales Justin" wrote:
Can you please not commit before we discuss this on the MC dev forum?
(currently the discussion is on the MC user forum, and you haven't given me any valid arguments)

Valid argument is here.

[ropalka]

"alesj" wrote:

* deployer stages have to be comparable

Why?
That's why they are called stages and they are already comparable - how else do you think they fit into our state machine. ;-)

Show me the code how would you compare two DeploymentStage instances?
Rule no. 1: Always implement comparable if your object instances represents natural order.

"alesj" wrote:

* deployers have to be sorted on deployers chain retrieval (i.e. in method protected synchronized List<Deployer> getDeployersList(String stageName))

Why?
Based on what?
This would be complete waste of time/performance.

I'd use the same trick Java iterators detect ConcurrentModificationException ;)
But not for throwing the exception, but for sorting the deployes chain only when necessary.

"alesj" wrote:

* sorting agorithm have to take stages, inputs, outputs and relative order into account

Why/how stages?
All the rest is already taken into account.

Because deployers in previous stages can provide outputs that are required in next stages deployers (in their inputs).

"alesj" wrote:

* different sorting algorithm have to be implemented (domino is wrong because two deployers are not comparable in general)

Sure.
But re-read my previous post:

"alesj" wrote:

If we have to resort to name comparison it either means:
* the order of how we process attachments doesn't matter
* you are describing/ordering your deployers wrong == lack of info

Sorry, I don't understand this? Could you explain me what do you mean, please.

[alesj]

Valid argument is <a href="http://www.jboss.org/index.html?module=bb&op=viewtopic&t=157558&postdays=0&postorder=asc&start=10#4239932">here.</a>

Why is this valid?

That's just you being lazy - not using statistics. ;-)
All you need to do is to identify that there exist a deployer providing the info
later than you expect it in some other deployer.
How you fix is up to you; change the stages, actual impl, ...

But this definitely doesn't mean that the ordering algorithm is wrong.

[ropalka]

"alesj" wrote:

- it doesn't validate inputs

Validate how?

Order at runtime based on the actual attachment values?
Too much hassle for something that sounds useless in 99,9%.
Not to mention that it's not deterministic.

By validating the input I mean:
if there's deployer that has input = "attachment_name", then
there's deployer before current deployer that defines output = "attachment_name".
I'm not talking about validating the deployer attachments at runtime.

"alesj" wrote:

- it sorts deployers on insert, but should sort them after deployers chain initialization

What would you gain by this?

I mean, how can you ensure deployers are inserted in correct order to deployers chain during AS bootstrap? This question is asked with precondition we're validating the deployers inputs as specified above.

The fundamental question here is why do you think we shouldn't validate inputs?

[alesj]

Rule no. 1: Always implement comparable if your object instances represents natural order.

There is a better rule.
Only implement what you actually need.
There is no need to really compare stages, at least not atm.

But you could do it this way:
ControllerStateModel model = ...;
DeploymentStage ds1 = ...;
ControllerState cs1 = new ControllerState(ds1.getString());
DeploymentStage ds2 = ...;
ControllerState cs2 = new ControllerState(ds1.getString());
boolean isBefore = model.isBefore(cs1, cs2);

"alesj" wrote:

If we have to resort to name comparison it either means:
* the order of how we process attachments doesn't matter
* you are describing/ordering your deployers wrong == lack of info

Sorry, I don't understand this? Could you explain me what do you mean, please.

What don't you understand?

If I need A and output B, and then need C and output D,
why should I care how are they ordered?

But if I in both cases need A and output A, B,
where in 1st deployer I set A.someprop
but in 2nd deployer I use that A.someprop,
all I need to do is express this via proper input/output,
e.g. D1.output=A.someprop, D2.input=A.someprop

[alesj]

The fundamental question here is why do you think we shouldn't validate inputs?

I'm not completely saying we shouldn't do this,
I'm just saying does it really make sense, or are you just lazy? ;-)

"alesj" wrote:

The only reasonable thing here might be that we check any previous stages for inputs,
and potentially break (throw an exception) if some deployer requires an input from later output.

But this should be properly discussed first on MC dev forum,
as I'm still not convinced this is something we should do.
e.g. we might just call it lazy deployer developer, as (s)he didn't bother to properly check previous deployers via statistics

I'm willing to add this check for dev usage, but not in production (*).
e.g. turn the check flag on, run/test the boot, and see if some deployer is broken

(*) the boot is already slow, w/o "helping" the "lazy"

[ropalka]

"alesj" wrote:

Rule no. 1: Always implement comparable if your object instances represents natural order.

There is a better rule.
Only implement what you actually need.
There is no need to really compare stages, at least not atm.

Agreed

"alesj" wrote:

But you could do it this way:
ControllerStateModel model = ...;
DeploymentStage ds1 = ...;
ControllerState cs1 = new ControllerState(ds1.getString());
DeploymentStage ds2 = ...;
ControllerState cs2 = new ControllerState(ds1.getString());
boolean isBefore = model.isBefore(cs1, cs2);

IMHO ds1.compareTo(ds2) its more intuitive.
Your solution includes whole MC knowledge people usually don't have ;)

"alesj" wrote:

But if I in both cases need A and output A, B,
where in 1st deployer I set A.someprop
but in 2nd deployer I use that A.someprop,
all I need to do is express this via proper input/output,
e.g. D1.output=A.someprop, D2.input=A.someprop

Then tell me what's wrong with this? Where's the lack of info I didn't provide?

[alesj]

Then tell me what's wrong with <a href="http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4239756#4239756">this</a>? Where's the lack of info I didn't provide?

Dunno, it depends on what you're doing in those 3 deployers.
They all input JBWD and output JBWMD, which is completely valid,
if they don't interfere with each others JBWMD usage/setup/config.
But if they do, I very much doubt that there exist a simple
(read fast and declarative) algorithm that would order you that. ;-)

[ropalka]

"alesj" wrote:

Then tell me what's wrong with <a href="http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4239756#4239756">this</a>? Where's the lack of info I didn't provide?

Dunno, it depends on what you're doing in those 3 deployers.

Nothing special, I'm just trying to access "DeploymentType" in deployer3 and deployer4,
because only deploye2 is providing this output.? Do you still think I'm really so lazy?

[alesj]

Nothing special, I'm just trying to access "DeploymentType" in deployer3 and deployer4,
because only deploye2 is providing this output.?

Ah, it's me then that's being lazy/blind. :-(
And you as well for not shutting my mouth on the first post. :-)
You should simply point this fact when I closed the issue,
since the rest of the discussion really went in all sorts of wrong directions.

Do you still think I'm really so lazy?

The lazy was never meant as *you* being lazy, but in general. (hope you didn't take it personally ;-))
Specially with checking the existing deployers, which is very simple to do with statistics.

I'll now try to get to the bottom of the *real* issue. :-)

[ropalka]

"alesj" wrote:

And you as well for not shutting my mouth on the first post. :-)

Agreed

"alesj" wrote:
You should simply point this fact when I closed the issue,
since the rest of the discussion really went in all sorts of wrong directions.

Yes, it was really a long round, uuuf ;)

"alesj" wrote:

The lazy was never meant as *you* being lazy, but in general. (hope you didn't take it personally ;-))
Specially with checking the existing deployers, which is very simple to do with statistics.

I knew it's just a phrase.

"alesj" wrote:

I'll now try to get to the bottom of the *real* issue. :-)

IMHO it's the fact you're not validating the inputs (as mentioned so many times).
But new impl should take deployer stages into account, because domino sorting happens only on deployers on the same stage in current impl.

[alesj]

But new impl should take deployer stages into account, because domino sorting happens only on deployers on the same stage in current impl.

No, deployment stage is something different, and imho it shouldn't have anything to do with explicit ordering.

Like I said, the only logical thing for me is that dev phase check,
everything else is just laziness --> more performance suffering boot time. ;-(

[ropalka]

My idea how to fix it:

ad1) Put all deployers to the same collection (not list per stage like current impl)
ad2) on deployers chain retrieval
* check whether sorting needs to be done (yes if deployers were added/removed to/from the chain from last getDeployersList() method call)
* sort all deployers if change is detected only
ad3) Return all deployers belonging to requested deployers chain (getDeployersList() method call) from sorted list of deployers

NOTE: Sorting algorithm have to take deployment stages, inputs, outputs and relative order into account. Current algorithm based on comparing two deployers doesn't include all possible usecases and it ignores inputs and stages.

[ropalka]

"alesj" wrote:

No, deployment stage is something different, and imho it shouldn't have anything to do with explicit ordering.

What about this usecase?