14 Replies Latest reply on Mar 13, 2002 10:40 AM by Adrian Brock

    Juha - Invocation sucks which makes interceptors suck.

    Trevor Squires Novice

      Yes, you *did* say that you didn't care what sort of invocation object to use. I chose the one your interceptors wanted to get optimized method calls with minimum code changes (and remove myself as a bottleneck :D).

      However, that style of invocation object sucks. I realised that it is going to break David J's attribute interceptor because he wants to intercept setAttributes().

      IMO you should be able to intercept everything in DynamicMBean and NotificationBroadcaster. This has been discussed in another thread but you didn't contribute. http://main.jboss.org/thread.jsp?forum=63&thread=9672

      So, I want to change the Invocation object in two ways:

      1. Payload changes

      I want the payload to be

      String invocationName
      Object[] invocationArgs
      String[] invocationSignature

      That's really no different from the current payload. However, I don't want to overload the payload based on TYPE/IMPACT as it is now. Instead I want to properly wrap an arbitrary invocation in the payload.

      For DynamicMBean.invoke(action, params, signature) the payload would be:

      invocationName: invoke
      invocationArgs: Object[] {action, params, signature}
      invocationSignature: String[] {String.class.getName(),Object[].class.getName(),String[].class.getName()};

      For DynamicMBean.getAttribute(attribute) it would look like:

      invocationName: getAttribute
      invocationArgs: Object[] {attribute}
      invocationSignature: String[] {String.class.getName()}

      Constants for DynamicMBean and NotificationBroadcaster invocationNames and invocationSignatures should go into a seperate Invocations or InvocationConstants class.

      2. I would like to support decoupled interceptors. I.e. where the interceptors do not do getNext().invoke() to pass the invocation on, they do an invocation.invoke().

      IMHO change 1 is a baseline requirement while change 2 is something that seems sensible.

      Trevor

        • 1. Re: Juha - Invocation sucks which makes interceptors suck.
          Juha Lindfors Master

          > However, that style of invocation object sucks. I
          > realised that it is going to break David J's
          > attribute interceptor because he wants to intercept
          > setAttributes().

          Why do you want to intercept a setAttributes() call instead of individual setAttribute(blah blah) call (which the setAttributes() delegates to)?

          You will need to check for instance the credentials for each write operation anyway in a security interceptor, so why does there need to be a way to intercept the aggregated call?


          >
          > IMO you should be able to intercept everything in
          > DynamicMBean and NotificationBroadcaster.

          yes, but why do you need to intercept the aggregated call instead of just intercepting the individual ones?

          -- Juha

          • 2. Re: Juha - Invocation sucks which makes interceptors suck.
            Juha Lindfors Master

            > 1. Payload changes
            >
            > I want the payload to be
            >
            > String invocationName
            > Object[] invocationArgs
            > String[] invocationSignature
            >
            > That's really no different from the current payload.
            > However, I don't want to overload the payload based
            > on TYPE/IMPACT as it is now. Instead I want to
            > properly wrap an arbitrary invocation in the
            > payload.

            ok, but do this for 1.1 version (I'm going to branch for beta as soon as I can). The payload also needs to be able to carry any arbitrary context information (whether in an object array, hashmap, I don't really care). This will become more clear once I get the connectors in.

            > 2. I would like to support decoupled interceptors.
            > I.e. where the interceptors do not do
            > getNext().invoke() to pass the invocation on, they
            > do an invocation.invoke().
            >
            > IMHO change 1 is a baseline requirement while change
            > 2 is something that seems sensible.

            ok, but again this is work for 1.1 as there is no immediate new functionality that this change adds.

            -- Juha


            • 3. Re: Juha - Invocation sucks which makes interceptors suck.
              Trevor Squires Novice


              > Why do you want to intercept a setAttributes() call
              > instead of individual setAttribute(blah blah) call
              > (which the setAttributes() delegates to)?
              >
              > You will need to check for instance the credentials
              > for each write operation anyway in a security
              > interceptor, so why does there need to be a way to
              > intercept the aggregated call?

              Yes that is true. However I recall seeing a post from David on the DEV list about expecting to intercept prior to the setAttributes() being expanded out.

              I *think* he wants to see if an attribute is of interest to his mbean lifecyclethingy interceptor. If it is he wants to take action and then let all of the attributes go through, leaving the MBean in a consistent state.

              Trevor

              • 4. Re: Juha - Invocation sucks which makes interceptors suck.
                Trevor Squires Novice

                > ok, but do this for 1.1 version (I'm going to branch
                > for beta as soon as I can).

                Ok then.

                > The payload also needs to
                > be able to carry any arbitrary context information
                > (whether in an object array, hashmap, I don't really
                > care). This will become more clear once I get the
                > connectors in.

                Well, in the interest of changing one thing at a time, I didn't mention context. As a placeholder I'd also like it to contain the ObjectName but that's for later.

                Trevor

                • 5. Re: Juha - Invocation sucks which makes interceptors suck.
                  Juha Lindfors Master

                  > Well, in the interest of changing one thing at a time

                  ok ;)

                  -- Juha

                  • 6. Re: Juha - Invocation sucks which makes interceptors suck.
                    David Jencks Master

                    "I *think* he wants to see if an attribute is of interest to his mbean lifecyclethingy interceptor. If it is he wants to take action and then let all of the attributes go through, leaving the MBean in a consistent state."


                    Exactly.

                    David

                    • 7. Re: Juha - Invocation sucks which makes interceptors suck.
                      Trevor Squires Novice

                      > ok, but do this for 1.1 version (I'm going to branch
                      > for beta as soon as I can). The payload also needs to
                      > be able to carry any arbitrary context information
                      > (whether in an object array, hashmap, I don't really
                      > care). This will become more clear once I get the
                      > connectors in.

                      Juha - I've been pondering this for a few days now.

                      I don't know what my boundaries are on this issue so I don't see how I can commit anything. Conciously or not, I'm sure you've formulated the precise way you think this should all fit together (despite saying you don't really care).

                      I am *not* bitching or whining here. I'm just making it clear that I'm backing away from this because I don't want to commit anything which might distort what you're working towards.

                      Trev

                      • 9. Re: Juha - Invocation sucks which makes interceptors suck.
                        Trevor Squires Novice

                        Thanks Adrian,

                        Payload is part of it but more fundamental are the intertwined issues of what's interceptable and what you expose (and to whom) that will accept the payload to give to the invocation chain.

                        This isn't really an issue of finding a technical solution. It's more that I have been approaching the solution from a perspective of "what makes sense". Juha undoubtedly has an idea of "what I need".

                        As such, my perspective is guaranteed to be inferior - not necessarily in a technical sense but in a relevance sense.

                        Trev

                        • 10. Re: Juha - Invocation sucks which makes interceptors suck.
                          Adrian Brock Master

                          From my point of view, I would say allow everything
                          to be intercepted. Then if interception isn't required,
                          the only interceptor is the target.

                          I would also expose as much as possible.
                          When coding an interceptor you are taking over the
                          responsibility of implementation. It is your fault if
                          you break it ;-)

                          Of course, I haven't looked at the implications in the
                          same depth that you probably have.

                          As a concrete example, I want to put interceptors in
                          the MBeanRegistry for register/unregisterMBean and
                          implement most of it as interceptors. This allows
                          flexible customization of the registration process,
                          without reimplementing the entire registry.
                          For registration, my payload is likely to be the MBean's
                          capabilities and the registered object name.
                          I don't feel any need to "protect" this data. What should
                          be protected is the interceptor stack's configuration.

                          Regards,
                          Adrian

                          • 11. Re: Juha - Invocation sucks which makes interceptors suck.
                            Trevor Squires Novice

                            Adrian,

                            I already went down a similar route. In fact at one stage I went so far that the agent (mbeanserverimpl) was an incredibly thin layer that did everything through invocation dispatchers.

                            I went completely off the scale - absolutely *everything* was interceptable and the responsibility for managing the invocation path (the expression of the MBean's capabilties - i.e. the invocation dispatcher, interceptors and invokable at the end) for a given MBean could be delegated to third party MBeans/POJOs (which in turn could use interceptors to protect themselves :D ).

                            Despite it looking cool how the agent had exploded out into a bunch of MBeans, unadorned DynamicMBeans ended up being slightly slower than the RI.

                            It also had the unfortunate side effect that anything taking direct advantage of the agent's interceptor architecture would be locked-in to JBossMX.

                            Ultimately what I did is just waffle. There is a middle ground but I don't see it as easy to find given my perspective.

                            And yes I think I have got a handle on the depth of the issues but as I write this I realise that *you're* better qualified to write this code. Adrian, make it do "what you need" and it is guaranteed to be correct.

                            Irrelevant code leads to anger, anger leads to hate... ;-)

                            Trev

                            • 12. Re: Juha - Invocation sucks which makes interceptors suck.
                              Adrian Brock Master

                              What do you have so far?

                              I remember reading JMX1.5 was going to specify
                              how to do this, but only for ModelMBeans I think?

                              It's obvious intercepting Dynamic MBeans is slower, but
                              why when unadorned?

                              I'll have a read of the commited code, I haven't looked
                              at this part for a couple of weeks :-)

                              Regards,
                              Adrian

                              • 13. Re: Juha - Invocation sucks which makes interceptors suck.
                                Trevor Squires Novice

                                > What do you have so far?
                                >
                                > I remember reading JMX1.5 was going to specify
                                > how to do this, but only for ModelMBeans I think?

                                Don't know, I thought it was for everything but I might be confusing it with the layered MBeanServers.

                                >
                                > It's obvious intercepting Dynamic MBeans is slower,
                                > but
                                > why when unadorned?

                                Because *everything* was interecepted. The agent never spoke directly to MBeans. I totally went off on one where the distinction between XMBeans and the agent was completely blurred. Depending on the angle you looked at it, either XMBeans were the agent or the agent was XMBeans.

                                For XMBeans this, of course, meant no overhead at all because they were first class citizens of the agent. For everything else there was a price to pay.

                                >
                                > I'll have a read of the commited code, I haven't
                                > looked
                                > at this part for a couple of weeks :-)

                                There is nothing in the codebase related to what I did. Even after scaling back my efforts, the remaining changes were too pervasive and it was hurting everyone else to wait for me to complete it.

                                XMBeans are the only thing in the agent that can take advantage of interceptors. If you want interceptors on the registry you'll have to hide it behind an XMBean. It makes sense given that you only want to pay for what you need.

                                Adrian, it was fun to work on and it's fun to talk about 'cause I thought it was neat. But seriously, I only wanted to find some way to remove myself from this task without sounding like I was pi$$ed off.

                                Every time I think about interceptors and the invocation path I go off on one and that'll just put me right back where I was when I (rightfully) got bitch-slapped by Juha.

                                Trev

                                • 14. Re: Juha - Invocation sucks which makes interceptors suck.
                                  Adrian Brock Master

                                  Ok,

                                  Just leave it as XMBeans. We can use what you got to
                                  build the rest if Juha agrees.

                                  The only concrete example of customized registration at
                                  the moment is a security interceptor.
                                  If I need anything else, I can build on what you've done.
                                  I wasn't planning on doing this until I'd finished
                                  testing 1.0 anyway. ;-)

                                  PS. I checked JMX1.5, it is interceptors for everything
                                  between the agent and the MBean. It quotes CORBA as the
                                  example.

                                  Regards,
                                  Adrian