Deployers Ordering Issue| JBoss.org Content Archive (Read Only)

15. Re: Deployers Ordering Issue

alesj Jun 25, 2009 9:09 AM (in response to ropalka)

"richard.opalka@jboss.com" wrote:

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

Nah, changing stages automagically is something you don't wanna do,
even though the order might appear to be wrong.
I would expect the stage to be set on purpose.
e.g. POST_CL, since I need ClassLoader to be present,
hence I don't wanna that my order changes based on someone else's illegal input/output

"richard.opalka@jboss.com" wrote:

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.

We don't ignore inputs --> inputs == Dots::head in Dominoes

16. Re: Deployers Ordering Issue

ropalka Jun 25, 2009 9:15 AM (in response to ropalka)

I'll let you understand the problem first Ales.
Then we'll discuss this (to prevent another long round) ;)
I just brain dumped my current thoughts to this forum to have it stored somewhere
before I'll return to other issues on my plate.

17. Re: Deployers Ordering Issue

alesj Jun 25, 2009 9:15 AM (in response to ropalka)

"richard.opalka@jboss.com" wrote:
"alesj" wrote:

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

What about <a href="http://www.jboss.org/index.html?module=bb&op=viewtopic&t=157558&postdays=0&postorder=asc&start=10#4239932">this</a> usecase?

Nope, not valid.
You're not seeing the whole picture (see my use case). :-)

So you're saying your use case should move Deployer2 into PRE_REAL?
What about if this is PRE_CL, whereas my Deployer2 was explicitly set to POST_CL - for pretty valid I-Need-ClassLoader reasons?
How would you generically/declaratively handle this?

18. Re: Deployers Ordering Issue

ropalka Jun 25, 2009 9:55 AM (in response to ropalka)

"alesj" wrote:

So you're saying your use case should move Deployer2 into PRE_REAL?

I'm not saying Deployer2 should be moved to PRE_REAL.
I'm saying such usecase should be rejected and exception must be thrown ;)

Generally speaking I don't need stages to be included in comparison.
But to create the correct fix of this issue, not just hotfix, stages needs to be taken into account as well and such wrong usecases (as defined above) have to be detected.

19. Re: Deployers Ordering Issue

ropalka Jun 25, 2009 10:02 AM (in response to ropalka)

"alesj" wrote:
"richard.opalka@jboss.com" wrote:
"alesj" wrote:

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

What about <a href="http://www.jboss.org/index.html?module=bb&op=viewtopic&t=157558&postdays=0&postorder=asc&start=10#4239932">this</a> usecase?

Nope, not valid.

Just to clarify my stay - Of course, this is not valid!
This is exactly what I'm trying to say.
What I'm saying is: Such wrong usecase isn't detected. It have to be detected and exception have to be thrown :(

20. Re: Deployers Ordering Issue

ropalka Jun 25, 2009 10:04 AM (in response to ropalka)

"richard.opalka@jboss.com" wrote:
<b>Such wrong usecase isn't detected. It have to be detected and exception have to be thrown :(</b>

And it won't be detected untill deployment stages will not be taken into account.

21. Re: Deployers Ordering Issue

alesj Jun 25, 2009 10:14 AM (in response to ropalka)

Although thinking about it.
I actually don't see why it wouldn't be valid.

Perhaps you really want what is stated there,
that in that particular stage, you have the explicit input/output ordering.
Regardless that the stages don't match,
e.g. output coming in later stage than the input
whereas this would come in play when new deployer with matching input/output is added

22. Re: Deployers Ordering Issue

ropalka Jun 25, 2009 10:25 AM (in response to ropalka)

"alesj" wrote:
Although thinking about it.
I actually don't see why it wouldn't be valid.

Perhaps you really want what is stated there,
that in that particular stage, you have the explicit input/output ordering.
Regardless that the stages don't match,
e.g. output coming in later stage than the input
<b>whereas this would come in play when new deployer with matching input/output is added</b>

This is exactly the reason why I'm saying:
ad1) You can't validate properly deployer dependencies/requirements in insert phase (domino algorithm tries to do it - wrongly)
ad2) You can do it properly only in method DeployersImpl.getDeployersList(String stageName) (i.e. sort lazily on retrieval and detect errors here and not in insert phase)

It's because you don't know the order in which deployers will be added to deployers chain. IOW you can't validate/sort untill you have full chain.

23. Re: Deployers Ordering Issue

alesj Jun 25, 2009 12:15 PM (in response to ropalka)

The only chain that is relevant is the per stage chain.

I'm now not even convinced that we need any dev check
as I suggested we do before - as "split" stage input/output is completely valid.
A case of that not being valid is too much impl detail and not a generic issue.

24. Re: Deployers Ordering Issue

ropalka Jun 26, 2009 1:25 AM (in response to ropalka)

"alesj" wrote:
as "split" stage input/output is completely valid.
A case of that not being valid is too much impl detail and not a generic issue.

Sorry, but I absolutely disagree with you :( Do things properly otherwise don't do it at all!

25. Re: Deployers Ordering Issue

alesj Jun 26, 2009 2:45 AM (in response to ropalka)

"richard.opalka@jboss.com" wrote:

Sorry, but I absolutely disagree with you :( Do things properly otherwise don't do it at all!

Apart from the bug in pass-through deployers,
you still haven't given me any good explanation on why this is not OK.

You can disagree all you want, but until I'm convinced what you're saying is valid,
things are gonna stay as they are, and please do *not* change+commit your code under MC projects.
I'm not randomly changing your code, so I would appreciate if you're not changing mine as well.

If the ordering doesn't exactly fit your needs, this doesn't always mean that it doesn't fit others as well.
We can always agree on extra hooks, that would help you validate things,
but some more drastic change might break too many things for us in existing AS deployers.
And mocking around with stages definitely sounds like asking for trouble.

26. Re: Deployers Ordering Issue

ropalka Jun 26, 2009 3:20 AM (in response to ropalka)

"richard.opalka@jboss.com" wrote:
Sorry, but I absolutely disagree with you :( Do things properly otherwise don't do it at all!

I'm taking these spicy words back. I was absolutely wrong! Your Domino sorting algorithm and the fix you did is correct Ales;) I highly apologize.

27. Re: Deployers Ordering Issue

ropalka Jun 26, 2009 3:27 AM (in response to ropalka)

"alesj" wrote:

Apart from the bug in pass-through deployers,
you still haven't given me any good explanation on why this is not OK.

I'm taking it back. This is OK. I already understood the domino sorting algorithm and thus see your point of view on this problem.

"alesj" wrote:

You can disagree all you want, but until I'm convinced what you're saying is valid,
things are gonna stay as they are, and please do *not* change+commit your code under MC projects.
I'm not randomly changing your code, so I would appreciate if you're not changing mine as well.

Sorry for my commits. I was so sure I'm right and Domino sorting is so wrong ;)

"alesj" wrote:
If the ordering doesn't exactly fit your needs, this doesn't always mean that it doesn't fit others as well.

Your fix is correct. Now all my use cases are working as expected. Thanks for the fix Ales!

"alesj" wrote:

We can always agree on extra hooks, that would help you validate things,
but some more drastic change might break too many things for us in existing AS deployers.
And mocking around with stages definitely sounds like asking for trouble.

Yes I know. You're right.

28. Re: Deployers Ordering Issue

ropalka Jun 26, 2009 3:32 AM (in response to ropalka)

"alesj" wrote:

Please do *not* change+commit your code under MC projects.
I'm not randomly changing your code, so I would appreciate if you're not changing mine as well.

I won't but I hope I can always commit the tests reproducing reported MC issues ;)
Do you agree?

29. Re: Deployers Ordering Issue

alesj Jun 26, 2009 3:48 AM (in response to ropalka)

"richard.opalka@jboss.com" wrote:

I won't but I hope I can always commit the tests reproducing reported MC issues ;)
Do you agree?

Definitely.

Just not the likes of changes made to DeploymentStage(s),
where we agreed we don't need them atm, but they still ended up in commits. ;-)