7 Replies Latest reply on Jan 31, 2010 3:51 AM by timfox

    Why does QueueImpl:expire() have...

    timfox
      ... a call to storageManager.completeOperations(); in it
        • 1. Re: Why does QueueImpl:expire() have...
          clebert.suconic
          It slipped through my eyes and it was forgotten there.
          • 2. Re: Why does QueueImpl:expire() have...
            clebert.suconic

            It is harmless though...

             

            completeOperation is not doing anything ATM. I will look into removing it.

             

             

            I believe it's being used as a hook for tests (from what I have seen now). I will look into removing this operation.

             

             

            We only need waitOnOperations (for management operatoins and GroupID) and clearContext.

            • 3. Re: Why does QueueImpl:expire() have...
              timfox

              Actually it's not harmless.

               

              The call to public static OperationContext getContext(final ExecutorFactory executorFactory) causes a thread local lookup and the creation of a context if it does not exist.

               

              All this contributes to reduction in performance e.g. for sending non persistent messages, and a contribution to excessive cpu utilisation.

               

              Also ServerSessionPacketHandler sendResponse gets called and does stuff even for non persistent messages with no response so is a waste of time.

              • 4. Re: Why does QueueImpl:expire() have...
                clebert.suconic

                Also ServerSessionPacketHandler sendResponse gets called and does stuff even for non persistent messages with no response so is a waste of time.

                 

                sendResponse is also calling confirmations, which you also need to do on NonPersistentMessages.

                 

                 

                I don't think an expiry call would be excessively used at our current benchmarks,  as i said anyway that method needs to go away.

                • 5. Re: Why does QueueImpl:expire() have...
                  timfox

                  "sendResponse is also calling confirmations, which you also need to do on NonPersistentMessages."

                   

                  Actually, you only need to call confirmations if confirmation-window-size != -1.

                   

                  The default value is -1. So in most cases you do not have to call it.

                  • 6. Re: Why does QueueImpl:expire() have...
                    clebert.suconic

                    Actually, you only need to call confirmations if confirmation-window-size != -1.

                     

                    The default value is -1. So in most cases you do not have to call it.

                     

                     

                    ok, I will add a check on window-size and packet != null on the call to sendResponse. I will do it as part of my TODO when removing that method.

                    • 7. Re: Why does QueueImpl:expire() have...
                      timfox

                      The completeOperations() method should be removed if it's not used anywhere in code.

                       

                      If only tests are using it, then they should be changed to not use it. It's not good to have methods only used in tests.

                       

                      Can you add a JIRA for this if you haven't done so already, so it doesn't get forgotten.