1 2 Previous Next 15 Replies Latest reply on May 24, 2006 4:50 PM by weston.price

    JBAS-1241 Background Pool Validation

    weston.price

      For http://jira.jboss.com/jira/browse/JBAS-1241:

      This issue popped up the priority stack recently. In looking at this, I propose the following:

      Modify BaseWrapperManagedConnectionFactory to implement the ValdatingConnectionFactory interface from the 1.5 spec. The method exposed is:

      Set getInvalidConnections(Set managedConnecdtions);

      Add validateConnections() method to InternalManagedConnectionPool that is called by an 'external' validating background thread (more on this in a bit).

      The BaseWrapperMCF should validate the set of MC's calling checkValid() much in the same manner as is done in the match method returning the invalid MC's that are then destroyed.

      The real issue I see with this is efficiency. The interface takes a set of MC's, returning the invalid set. Taken at face value this requires 2 passes through the list of connection listeners. One to get the MC's and the next to find the listeners to destroy. Since this has to be synchornized, this could get to be a real bottle neck, background thread or not.

      As an optimization, a JBossValidatingCF interface could be added that took a let of ConnectionListeners and returned the ones that needed to be removed directly. The invalid connections could then be destroyed immiedately.

      We could either add an element to the XML to tell the pool to look for this, set a flag in the constructor, do it dynamically etc, etc.

      As far as a background thread, I don't like using the IdleRemover to do this. I think it should be a seperated thread BackgroundValidator or something like that. Right now the IdleRemove waits on the idle timeout interval. At the very least this would have to be modified, or add another remover to do the validation. If we are going to this, let's just clarify it as another class.

        • 1. Re: JBAS-1241 Background Pool Validation

          The way I see this change is:

          1) Add new parameters to the connection pool for the background
          checking frequency
          2) Warn if frequency configured and it doesn't implement the JCA 1.5
          optional interface
          3) Change InternalManagedConnectionPool to have a validateConnections
          like you said
          4) In validateConnections() - pseudo code (be careful with error handling)

          if (permits.attempt()) // Get a single permit, don't worry about it if all the permits are in use
          {
           // Whether we destroyed a connection
           boolean destroyed = false;
           try
           {
           while (true)
           {
           ConnectionListener cl = null;
           // Temporarily make the first connection that needs checking
           // unavailable to applications
           synchronized (cls)
           {
           cl = removeFirstClNotCheckedWithinFrequency();
           }
           // We've checked them all
           if (cl == null)
           break;
           // Check the single connection
           try
           {
           Set set = Collections.singleton(cl.getManagedConnection());
           set = ((ValidatingConnectionFactory) mcf).getInvalidConnections(set);
          
           // This connection failed
           if (set != null && set.size() > 0)
           {
           // If the checking already invoked connectionErrorOccured
           // don't bother warning
           if (cl.state != DESTROYED)
           log.warn("....");
          
           // Destroy the failed connection
           destroy(cl);
           destroyed == true;
           }
           }
           finally
           {
           // Its important that the MRU list is maintained
           // for the idle removal alogorithm to work properly
           synchronized (cls)
           {
           putItBackAccordingMRU(cl);
           }
           }
           }
           }
           finally
           {
           // Release our permit
           permits.release();
          
           // We destroyed something, check the minimum.
           if (destroyed && shutdown.get() == false && poolParams.minSize > 0)
           PoolFiller.fillPool(this);
           }
          }
          


          • 2. Re: JBAS-1241 Background Pool Validation

            The JDBC resource adapter should change to implement the JCA1.5 api.
            It needs an extra parameter to say don't do the validation check
            for every matchManagedConnections().

            This can be automatically set to false when the frequency is specified,
            if the user doesn't specify it explicitly.

            I don't understand why you need a JBoss specific api?

            • 3. Re: JBAS-1241 Background Pool Validation

               

              "weston.price@jboss.com" wrote:

              As far as a background thread, I don't like using the IdleRemover to do this. I think it should be a seperated thread BackgroundValidator or something like that. Right now the IdleRemove waits on the idle timeout interval. At the very least this would have to be modified, or add another remover to do the validation. If we are going to this, let's just clarify it as another class.


              This really needs tidying up. There are already two threads
              1) IdleRemover
              2) PoolFiller

              There should be a single "scheduler" for all these background tasks.
              When running inside JBossAS it should use whatever it defines as the
              background task scheduler (currently this doesn't exist but IIRC there
              is a feature request somewhere).
              But this can be left to a future task (as long as it gets done :-)

              • 4. Re: JBAS-1241 Background Pool Validation

                The reason I would implement it the way I sketched out, is because it doesn't
                lock out the entire pool from the application threads while you are
                checking a single connection.

                Another possible improvement would be to remove the automatic
                valid connection checking from the resource adapter's
                matchManagedConnections() and instead invoke the optional
                interface from the pool based on connection manager configuration.
                IMHO this is a much cleaner way to do it. ;-)

                • 5. Re: JBAS-1241 Background Pool Validation
                  weston.price

                  Weird...I think I am actually beginning to think like you...

                  The JBoss specific API was largely due to offering an optimized version of the 'checking' the new explanation removes this need.

                  • 6. Re: JBAS-1241 Background Pool Validation

                     

                    "weston.price@jboss.com" wrote:
                    Weird...I think I am actually beginning to think like you...


                    Oh dear! :-)

                    • 7. Re: JBAS-1241 Background Pool Validation
                      weston.price

                      Question on branch/version:

                      The priority of this popped up the stack due to some support stuff. Should we put this in as a patch for a specific customer (current version 4.0.2) or should it go into a more current release and then we backport etc, etc.

                      The only reason I ask is because I am getting asked about it.

                      • 8. Re: JBAS-1241 Background Pool Validation

                        You do it in the main branches first then backport to a patch branch.
                        1) Otherwise, it will get lost.
                        2) You need others to be using/testing this feature which means
                        it should be on the main branch.

                        • 9. Re: JBAS-1241 Background Pool Validation
                          weston.price

                          The pseudo code was about what I had, however on the removeFirstCheckedNotWithinFrequency() etc, etc. I am assuming we have to maintain some sort of book keeping on which CL's have been checked and then reset the state after the validation completes.

                          Correct?


                          • 10. Re: JBAS-1241 Background Pool Validation

                            I'd just add a "last validated time" to the ConnectionListener class.

                            • 11. Re: JBAS-1241 Background Pool Validation
                              weston.price

                              Weird...I just did that...oh man, this is creepy....

                              • 12. Re: JBAS-1241 Background Pool Validation
                                weston.price

                                On the thread/scheduling implementation, here is what I think should happen; again, this can be retrofitted later, but this does, as you pointed out need cleanup.

                                Currently each thread Idle,Filler merely invokes a method on the Internal Pool. This is fine, but should probably be refactored into a single JCARunnable type process that can be independently scheduled. For the Idle and Validation threads we would just need to specify the interval, and the actual method to call (new interface to get this method), PoolFiller could be fit into this model as well.

                                • 13. Re: JBAS-1241 Background Pool Validation

                                  This should really be on a separate thread and discussed
                                  as part of generic feature for all projects.
                                  Probably using a java.util.concurrent api?

                                  • 14. Re: JBAS-1241 Background Pool Validation
                                    weston.price

                                    This is where it would have been real, real handy to actually have a WorkManagement spec that wasn't bogged down with legality.

                                    1 2 Previous Next