Hi Shenglin
It's a step in the right direction, thanks, but JMXServer still needs
to be cleaned up quite a bit.
- as I said earlier you have 4 big functions basically duplicating
each other. We can't have a method per every attribute a given MBean
may have. Have a single function only, max two (one capturing all the
attributes, another one - dedicated to objectname/{value}).
- Introduce MBeanResource subresource instead of overloading MBean, it
will be cleaner and easier to work with
- as suggested earlier, update the domain MBean and/or MBeanAttribute
such that you could drop a big chunk of code in JMXServer where
individual attribute values are set, currently in getMbeanMap (if
(attrName.equals("service") then/else)). That is because a number of
attributes is basically unlimited, even though we have some well-known
ones
- getMBeanMap - this is so complex I can't understand what it does. I
do understand it returns a thread-safe Map - but I'm finding it
difficult to understand how the method achieves that. Please remove
all those synchronized blocks and have ConcurrentHashMap. Have atomic
integer counter, synchronized exlicitly (in a block) if you prefer,
and try to get a simple and effective function implemented. Don't make
the logic of that function dependent on a n
I don't understand why you use "list" for creating MBean hrefs;
- there's no need for having Mbean.id and MBean.href because
MBean.href is identifying a given MBean uniquely and thus you could
use that calculated href as a key.
Thanks, Sergey
On Tue, May 31, 2011 at 5:36 PM, Shenglin Qiu <dabaip...@hotmail.com> wrote:
>
> Hi Sergey:
>
> Project has been synced to Github:
>
> Browser:
> https://github.com/dabaipang/services
>
>
> Git address:
> https://dabaip...@github.com/dabaipang/services.git
> or
> g...@github.com:dabaipang/services.git
>
>
> Thank you.
>
>
> Regards:
> Shenglin Qiu
>
>
>
>> From: dabaip...@hotmail.com
>> To: sberyoz...@gmail.com
>> CC: dev@cxf.apache.org
>> Subject: RE: Expose MBeans in CXF
>> Date: Tue, 31 May 2011 11:23:38 -0400
>>
>>
>> Hi Sergey:
>>
>> > > Function 1: (0 - 5 is continuous, no gap)
>> > > Jmx Server:
>> > > sub-resource locator
>> > > http://localhost:8080/services/jmx/mbean/0
>> > > http://localhost:8080/services/jmx/mbean/1
>> >
>> > The id allocation (0, 1, etc) has to be thread safe
>>
>> Done with:
>> private synchronized Map<String, MBean> getMBeansMap(){
>> ....
>> }
>> My reason not using synchronized block but using synchronized method is:
>> this guarantees the order on mbeansMap's initialization, so when it's
>> called by a lot of requests, the followings will have to wait, unit the
>> first one finished, after that, following requests will see mbeansMap has
>> been initialized, and simply return mbeansMap without initializing it again.
>>
>>
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>> > or
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf
>> >
>> > should also work
>> >
>> > >
>>
>> http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>> is working.
>>
>> > > Due to the URL duplication with /mbean/{id}, I can't put
>> > > {searchtype}/{query}, so I put things like:
>> >
>> > Personally I'd prefer to keep a URI as minimal as possible.
>> >
>> > /mbean/{id}
>> > and
>> > {searchtype}/{query}
>> >
>> > do not duplicate each other given that /mbean/{id} is more specific
>> > than {searchtype}/{query}.
>> > Likewise /objectname/{objectname} is more specific than
>> > {searchtype}/{query}, so consider dropping /search/ part,
>> > we may have a dedicated /search handler later on.
>>
>> Done by dedicating each request per method:
>> @GET
>> @POST
>> @Path("/list")
>> public MBeanCollection list()
>>
>> @Path("/mbean/")
>> public MBean locateMBean()
>>
>>
>> @GET
>> @POST
>> @Path("/type/{query}")
>> public MBeanCollection searchMBeansByType(@PathParam("query") String
>> query)
>>
>>
>> @GET
>> @POST
>> @Path("/port/{query}")
>> public MBeanCollection searchMBeansByPort(@PathParam("query") String
>> query)
>>
>>
>> @GET
>> @POST
>> @Path("/service/{query}")
>> public MBeanCollection searchMBeansByService(@PathParam("query") String
>> query)
>>
>>
>> @GET
>> @POST
>> @Path("/objectname/{objectname}")
>> public MBeanCollection getComponent(@PathParam("objectname") String
>> objectname)
>>
>> I remembered you mentioned to handle Exceptions once, currently I am still
>> 'throws' -ing them out, I will make the try catch block.
>> Should I also add a new object which display the error or simply return
>> empty?
>>
>>
>> Thank you very much.
>>
>>
>> Regards:
>> Shenglin Qiu
>>
>>
>> > Date: Tue, 31 May 2011 10:35:36 +0100
>> > Subject: Re: Expose MBeans in CXF
>> > From: sberyoz...@gmail.com
>> > To: dev@cxf.apache.org
>> >
>> > Hi Shenglin
>> >
>> > Good progress, some comments below
>> > >
>> > > Function 1: (0 - 5 is continuous, no gap)
>> > > Jmx Server:
>> > > sub-resource locator
>> > > http://localhost:8080/services/jmx/mbean/0
>> > > http://localhost:8080/services/jmx/mbean/1
>> >
>> > The id allocation (0, 1, etc) has to be thread safe
>> >
>> > >
>> > >
>> > > Function 2:
>> > > Note: search by bus.id, bus.id will be changed on every time server had
>> > > been
>> > > bounced.
>> > > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,*
>> > > ...
>> > that is ok, a user does not have to specify them as you demonstrated
>> > with queries like
>> >
>> > > http://localhost:8080/services/jmx/objectname/org.apache.cxf:port=%22CustomerServiceImpl%22,*
>> > I guess
>> >
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>> > or
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf
>> >
>> > should also work
>> >
>> > >
>> > >
>> > > I am only now use 2 methods to fulfill:
>> > > 1 method for service/{serivce}, port/{port}, type/{type}
>> > > 1 method for objectname/{objectname}
>> >
>> > OK
>> >
>> > >
>> > > Due to the URL duplication with /mbean/{id}, I can't put
>> > > {searchtype}/{query}, so I put things like:
>> >
>> > Personally I'd prefer to keep a URI as minimal as possible.
>> >
>> > /mbean/{id}
>> > and
>> > {searchtype}/{query}
>> >
>> > do not duplicate each other given that /mbean/{id} is more specific
>> > than {searchtype}/{query}.
>> > Likewise /objectname/{objectname} is more specific than
>> > {searchtype}/{query}, so consider dropping /search/ part,
>> > we may have a dedicated /search handler later on.
>> >
>> > The other thing I forgot to mention, please remove all those
>> > System.out.println, ping me please if you need some help with setting
>> > up a remote debugging session
>> >
>> > Thanks, Sergey
>> >
>> > >
>> > > Thank you so much!
>> > >
>> > > Regards:
>> > > Shenglin Qiu
>> > >
>>
>
--
Sergey Beryozkin
Application Integration Division of Talend
http://sberyozkin.blogspot.com
