11 Replies Latest reply on Mar 2, 2010 11:07 AM by Ales Justin

    JBDEPLOY-226 - DeployerClient change extension

    Adrian Brock Master

      I've added a new mixin interface to implement this feature.

      https://svn.jboss.org/repos/jbossas/projects/jboss-deployers/trunk/deployers-client-spi/src/main/java/org/jboss/deployers/client/spi/DeployerClientChangeExt.java

      since I want to be able to do this as well in some of the stuff I'm doing.

       

      There's also a mixin interface on DeployersImpl, so it won't work for Thomas unless

      he changes his wangy wrapper to delegate the new method.

      If the Deployers doesn't implement the new mixin interface, it falls back to doing

      the deployments one by one, i.e. the old behaviour.

       

      I've left the JIRA open in case we want to add some other methods?

       

      One possiblity would be to have a

       

      addChange(DeploymentStage, String name);

      processChanges();

       

      type api that let you move deployments to different stages at the same time.

      But I'm not sure how useful that would be and the implementation would be a bit more complicated

      and probably less efficient? :-)

       

      If everybody is happy with just this one new method we can close the JIRA.

        • 1. Re: JBDEPLOY-226 - DeployerClient change extension
          Ales Justin Master
          since I want to be able to do this as well in some of the stuff I'm doing.

          Related to JBKERNEL-54 / JBOSGI-151? :-)

          • 2. Re: JBDEPLOY-226 - DeployerClient change extension
            Adrian Brock Master
            alesj wrote:

             

            since I want to be able to do this as well in some of the stuff I'm doing.

            Related to JBKERNEL-54 / JBOSGI-151? :-)

             

            I don't see why resolving circular dependencies needs this feature?

            The problem with Thomas's implementation (if it is like what he has done elsewhere)

            is that he is doing a checkComplete() too often.

            He should check them all at the end, not during/after every single move. :-)

             

            No, it is to implement the refreshPackages() behaviour when using shutdownPolicy=GC

            I'm implementing it properly inside org.jboss.classloading.spi.dependency.ClassLoading

            where none OSGi users can use this feature as well.

             

            It's not strictly required, but width first processing of deployments is preferable for a

            number of reasons.

             


            • 3. Re: JBDEPLOY-226 - DeployerClient change extension
              Ales Justin Master

              I don't see why resolving circular dependencies needs this feature?

              The problem with Thomas's implementation (if it is like what he has done elsewhere)

              is that he is doing a checkComplete() too often.

              He should check them all at the end, not during/after every single move. :-)

              I doesn't.

               

              But it would definitely be good if it was the other way around - this feature needs circular resolving.

              Otherwise it doesn't make much sense, if the deployments are not actually moved into the state you wanted them to move.

              At least for the "valid" circular deployments -- which is often the case in OSGi -- according to TCK.

               

              I don't see how checking at the end would help. They would still not be in the "right" state.

              Or are you saying Controller::change(deploymentContext, state) already knows how to resolve this?

              e.g. deployment A imports some B's packages, and vice versa, and afterwards we would expect both of them in past Describe state

              (or how do valid OSGi circular depencies look like?)

              • 4. Re: JBDEPLOY-226 - DeployerClient change extension
                Adrian Brock Master

                I'm saying this feature is irrelevant to the problem.

                 

                Whether it can resolve circular references is a seperate issue for the dependency mechanism to spot.

                The order in which you change the state of things shouldn't make a difference.

                 

                If A depends upon B which then depends upon A all at stage 1, it does not matter whether you do:

                 

                change(A, stage1);

                change(A, stage2);

                change(B, stage1);

                change(B, stage2);

                checkComplete(A, B);

                 

                or

                 

                change(A, stage1);

                change(B, stage1);

                change(A, stage2);

                change(B, stage2);

                checkComplete(A, B);

                 

                The result should be the same.

                 

                If you add more checkComplete()s between the steps then obviously it is going to report an incomplete

                deployment until A and B are both at stage1.

                 

                change(A, stage1);

                checkComplete(A); // Oops, IncompleteDeploymentException - A depends B which is not in the correct state yet, so A is not at stage 1 either.


                • 5. Re: JBDEPLOY-226 - DeployerClient change extension
                  Ales Justin Master

                  I see we're on the same page -- this feature is just a nicer API, done width first.

                   

                  I was just wondering if you're tackling the circular references problem.

                  ((ab)using this thread to check up on you )

                  • 6. Re: JBDEPLOY-226 - DeployerClient change extension
                    Thomas Diesler Master

                    Yes, the additional checks for completeness seem to have been the problem.

                     

                    I now do this in PackageAdmin

                     

                          // Advance the bundles to stage CLASSLOADER and check at the end
                          advanceBundlesToClassloader(resolvableBundles);
                          try
                          {
                             DeployerClient deployerClient = getBundleManager().getDeployerClient();
                             deployerClient.checkComplete();
                          }
                          catch (DeploymentException ex)
                          {
                             log.error("Error resolving bundles: " + resolvableBundles, ex);
                             allResolved = false;
                          }
                    

                     

                    There is also an additional deployer which sets the bundle state to RESOLVED once the deployment reaches the CLASSLOADER stage.

                     

                    I resolved https://jira.jboss.org/jira/browse/JBOSGI-267

                    • 7. Re: JBDEPLOY-226 - DeployerClient change extension
                      Adrian Brock Master

                      adrian@jboss.org wrote:

                      I've left the JIRA open in case we want to add some other methods?

                       

                      I've also added a notion of bouncing a set of deployments since this is really the use case for the new api.

                       

                      bounce(DeploymentStages.DESCRIBE, true, deploymentNames);

                       

                      will move all the deployments listed that have proceed beyond the DESCRIBE stage back to the DESCRIBE stage

                      then return them to their original required stage.

                      i.e. it will re-resolve the classloader dependencies and re-create the classloaders.

                      • 8. Re: JBDEPLOY-226 - DeployerClient change extension
                        Adrian Brock Master

                        alesj wrote:

                         

                        I see we're on the same page -- this feature is just a nicer API, done width first.

                         

                        I was just wondering if you're tackling the circular references problem.

                        ((ab)using this thread to check up on you )

                        Nope. But that probably needs resolving (pun intended) in the jboss-dependency project?

                         

                        Currently a DependencyItem knows what it wants to resolve against and just checks whether it is in the correct state.

                        If however this leads to a circular dependency, nothing moves.

                         

                        e.g.

                        A depends upon B at CLASSLOADER

                        B depends upon A at CLASSLOADER

                         

                        For A,

                        ControllerContext context = controller.getContext(B, CLASSLOADER);

                        will return null and the same for B with respect to A.

                         

                        Instead, what is required is some api within the DependencyInfo/Item to say, well actually I will return a context because

                        the only thing that is stopping them moving is the circular dependency.

                         

                        Something like:

                         

                        RequirementDependencyItem extends AbstractDependencyItem

                        {

                           public boolean resolve(Controller)

                           {

                               Module module = resolveModule();

                               // Now we know what we want to resolve to

                               if (module != null)

                               {

                                  ControllerContext ctx = module.getControllerContext();

                                  putThisDependencyInASemiResolvedStateOnContext(ctx)

                                  boolean resolved = doesTheOtherContextAndItsTransientsHaveCompletelyUnresolvedDependenies(ctx, getDependentState()); // i.e. not resolved at all rather resolved or semi-resolved

                                  // etc.

                               }

                           }

                        }

                         

                        I can prototype this in the ClassLoading project, but I think it is a general requirement?

                         

                        NOTE: This feature is only possible for a dependency where the underlying model can handle circular dependencies.

                         

                        e.g. constructors on pojos can't do it

                        But the classloading can, since it can lazily wire the classloaders together using the ClassLoaderPolicyFactory on the DelegateLoader.

                        • 9. Re: JBDEPLOY-226 - DeployerClient change extension
                          Ales Justin Master

                          Nope. But that probably needs resolving (pun intended) in the jboss-dependency project?

                          I can prototype this in the ClassLoading project, but I think it is a general requirement?

                          What ever is easier for you.

                           

                          Although it sounds like it should be a general requirement,
                          I don't really see any other valid use case apart from classloading+deployments,
                          hence it could easily only exist in ClassLoading project.

                          Although you would still have to provide a couple of mixins in jboss-dependency?

                           

                          As you already mentioned, to make this generic,
                          dependency item itself would have to be somehow aware of what is valid or what's not.
                          e.g. the circular ctor case is not valid, but CL dependencies are

                          • 10. Re: JBDEPLOY-226 - DeployerClient change extension
                            Scott Stark Master
                            I can see the same circularity issue existing for profiles. I would try to address this generically.
                            • 11. Re: JBDEPLOY-226 - DeployerClient change extension
                              Ales Justin Master
                              I can see the same circularity issue existing for profiles. I would try to address this generically.

                              Profiles currently are not using MC state machine / component model, but we plan on using it?

                              If profiles are circular, perhaps that just means the those deployments need to be in the same profile?

                               

                              I'll let Adrian explain it more -- on what his plan on tackling this generically are? :-)