[Go-essp-tech] TDS Authorization issues
Estanislao Gonzalez
gonzalez at dkrz.de
Wed Feb 22 08:42:49 MST 2012
Hi Luca,
the solution looks great. And indeed it's better to code with semantics,
if it should be used as singleton, then code it that way. No harm done :-)
I've been following this behavior for quite some time... Though I think
I'm more concern about what tomcat does when the client closes the
connection... I just can't wait to upgrade it to see how it behaves...
Anyway, regarding having main classes for your personal testing in the
main branch, I don't really concur with it...
IMO you could extract a more general test and put it there, or use a new
directory src/local and ignore it for the repo. Again, IMHO if those
classes are only going to be useful to you, don't put them in the repo :-)
The problem is that the security has already too many classes, counting
all interfaces and factory... this makes more difficult to follow the
logic, and is precisely in the security module where the logic should be
as clear as possible to avoid adding security loop holes.
My 2c :-)
Estani
Am 22.02.2012 14:13, schrieb Cinquini, Luca (3880):
> Hi all,
> indeed I switched the SOAP invocations about a week ago to be multi-threaded, to work around the limitations in the apache SimpleHttpConnectionManager. This is after I saw in the logs an error message about "Maximum number of open connections reached".
>
> The SOAP invocations happen in two places:
>
> a) From the TDS authorization filter, which contacts the SAML Authorization service
> b) From the Authorization service, which contacts the attribute service
>
> a) is a filter initialized by Tomcat, so there should be only one instance
> b) is an instance instantiated by Spring, so there should also be only one instance
>
> So I think we should be ok... As the "devel" code goes into production with the new release (tomorrow, probably), we should be able to monitor if the problem disappear. I could also make the SOAPServiceClient class a Java singleton, just to make sure there's only one... infact, I'll do that today, just to be sure.
>
> So are we all good ?
>
> thanks, Luca
>
> P.S.: Estani, the classes you refer to are not real tests - they are main classes to "probe" the services. I don't think I should move them to the test directory - but let me know what you think...
>
> On Feb 22, 2012, at 5:41 AM, Estanislao Gonzalez wrote:
>
>> Damn, and I *though* I'll fetch the repo before checking anything... but
>> first just check it's indeed in security... and there's when I never got
>> back to it :-/
>>
>> In acy case... that's pretty new Feb 16th :-) But yeah... we are a week
>> too late :-)
>>
>> Thanks,
>> Estani
>> Am 22.02.2012 12:56, schrieb stephen.pascoe at stfc.ac.uk:
>>> I think Luca has already fixed it in git's devel branch.
>>>
>>> SAMLAuthorizationServiceFilterCollaborator instantiates SAMLAuthorizer (final) which instantiates SAMLServiceClient (final) which instantiates HttpClient. The collaborator is a singleton created from spring so I *think* there is only one SAMLServiceClient. In Git this instantiates HttpClient like this:
>>>
>>> public SOAPServiceClient() {
>>> HttpConnectionManager manager = new MultiThreadedHttpConnectionManager();
>>> manager.getParams().setConnectionTimeout(TIMEOUT);
>>> manager.getParams().setSoTimeout(TIMEOUT);
>>> client = new HttpClient(manager);
>>> }
>>>
>>> As usual you can't beat Luca to the punch :-)
>>>
>>> Cheers,
>>> Stephen.
>>>
>>> ---
>>> Stephen Pascoe +44 (0)1235 445980
>>> Centre of Environmental Data Archival
>>> STFC Rutherford Appleton Laboratory, Harwell Oxford, Didcot OX11 0QX, UK
>>>
>>>
>>> -----Original Message-----
>>> From: Estanislao Gonzalez [mailto:gonzalez at dkrz.de]
>>> Sent: 22 February 2012 11:45
>>> To: Nathan Wilhelmi; Cinquini, Luca (3880); Pascoe, Stephen (STFC,RAL,RALSP)
>>> Subject: Re: [Go-essp-tech] TDS Authorization issues
>>>
>>> Hi Stephen,
>>>
>>> I guess I should have added you to this email.
>>>
>>> Thanks,
>>> Estani
>>>
>>> Am 22.02.2012 10:35, schrieb Estanislao Gonzalez:
>>>> Hi Nate,
>>>>
>>>> thanks for this... indeed it looks like the problem I've been seen...
>>>>
>>>> Could you send us the patch?
>>>>
>>>> I've tried to see it myself, but it's really hard to see what gets
>>>> used where, with all the Spring wiring happening dynamically...
>>>> I've found only 5 non-util classes with synchronized methods in
>>>> them... and none of them in the SOAP area... I wonder if that part of
>>>> the code is multi-thread tolerant at all....
>>>>
>>>> I can't patch it myself without knowing what is really being done. How
>>>> have you done it Nate?
>>>> For instance, the problem is here:
>>>> public SOAPServiceClient() {
>>>> HttpConnectionManager manager = new
>>>> SimpleHttpConnectionManager();
>>>> manager.getParams().setConnectionTimeout(TIMEOUT);
>>>> manager.getParams().setSoTimeout(TIMEOUT);
>>>> client = new HttpClient(manager);
>>>>
>>>> }
>>>>
>>>> But SOAPServiceClient is not a singleton, so potentially we could have
>>>> multiple objects, is that true? If So, I'd think the connection
>>>> manager should be extracted to a static variable and made
>>>> multi-threaded. If not, then The whole class should be a singleton and
>>>> add the multi-threaded connection manager exactly there where it is...
>>>> but I can't find out how exactly is it being used (I assume the later,
>>>> as it's in a factory class... but then again the factory itself
>>>> (esg.security.authz.service.impl.SAMLAuthorizationFactoryImpl) is
>>>> neither a singleton nor static, so Spring could potentially be crating
>>>> multiple factories... very confusing :-/
>>>>
>>>> Anyway, Luca, I also saw many classes that look like test classes, but
>>>> are on the main branch... they make the code harder to read in my
>>>> opinion... if that' so, could you please refactor them to the test
>>>> source-code branch? I've found for example these while trying to
>>>> figure out where SOAPServiceClient is being used:
>>>>
>>>> esg.security.attr.main.SAMLAttributeServiceSOAPClient
>>>> esg.security.attr.main.SecureSAMLAttributeServiceSOAPClient
>>>> esg.security.authz.main.SAMLAuthorizationServiceSOAPClient
>>>> esg.security.authz.main.SecureSAMLAuthorizationServiceSOAPClient
>>>> esg.security.webclient.TestController
>>>>
>>>> They all have hard coded values and just a main method.
>>>>
>>>> Thanks,
>>>> Estani
>>>> Am 22.02.2012 03:55, schrieb Nathan Wilhelmi:
>>>>> Hi All,
>>>>>
>>>>> In debugging the issue I think we found a couple of issues on the node
>>>>> that would manifest themselves under load.
>>>>>
>>>>> HttpClient may not be closing connections, it depends on the load, GC,
>>>>> etc. This is a pretty good summary of the problem:
>>>>>
>>>>> http://fuyun.org/2009/09/connection-close-in-httpclient/
>>>>>
>>>>> Once we patched the SOAPServiceClient.class code to use a single
>>>>> instance the authorization calls began working again. We also found that
>>>>> the authorization would work reliably, albeit very slowly, if tomcat was
>>>>> only given a single thread.
>>>>>
>>>>> Estani, I believe this is likely the source of dangling sockets you
>>>>> noted in earlier emails.
>>>>>
>>>>>
>>>>> Thanks!
>>>>> -Nate
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 02/20/2012 02:20 PM, Cinquini, Luca (3880) wrote:
>>>>>> Hi Stephen,
>>>>>> we expect a release any day - Gavin needs to fix something one
>>>>>> the node manager, then we will build a release... we'll let you know...
>>>>>> thanks, Luca
>>>>>>
>>>>>> On Feb 20, 2012, at 8:49 AM,<stephen.pascoe at stfc.ac.uk> wrote:
>>>>>>
>>>>>>> Hi Luca,
>>>>>>>
>>>>>>> This is great timing -- I was just thinking about making some sort
>>>>>>> of cache to fix this AttributeService problem.
>>>>>>>
>>>>>>> So, I'm eager to move to this version. I have a partially-built
>>>>>>> test P2P node on which I last ran the script about 10 days ago.
>>>>>>> How should I go about installing this version with caching
>>>>>>> AuthzService? Our test P2P node is intended to either work in
>>>>>>> datanode-only mode or move swiftly to a full P2P node with index
>>>>>>> and web-fe, depending on how the deployment goes. My top priority
>>>>>>> is to make sure our production datanode is compatible with the P2P
>>>>>>> system.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Stephen.
>>>>>>>
>>>>>>> ---
>>>>>>> Stephen Pascoe +44 (0)1235 445980
>>>>>>> Centre of Environmental Data Archival
>>>>>>> STFC Rutherford Appleton Laboratory, Harwell Oxford, Didcot OX11
>>>>>>> 0QX, UK
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Cinquini, Luca (3880) [mailto:Luca.Cinquini at jpl.nasa.gov]
>>>>>>> Sent: 20 February 2012 14:40
>>>>>>> To: Pascoe, Stephen (STFC,RAL,RALSP)
>>>>>>> Cc: wilhelmi at ucar.edu; go-essp-tech at ucar.edu
>>>>>>> Subject: Re: [Go-essp-tech] TDS Authorization issues
>>>>>>>
>>>>>>> Hi Stephen,
>>>>>>> and you'll be happy to know that with the next release, there
>>>>>>> will be caching of user attributes by the authorization service -
>>>>>>> your most desired feature.
>>>>>>> This release will support the data-node-only configuration. I've
>>>>>>> been testing and it looks good, but I'd like another node to test
>>>>>>> it first before we ask more nodes to install it.
>>>>>>> thanks, Luca
>>>>>>>
>>>>>>> On Feb 20, 2012, at 3:38 AM,<stephen.pascoe at stfc.ac.uk> wrote:
>>>>>>>
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> For some reason I've got these git URLs for esgf-security but I
>>>>>>>> think esg-repo.llnl.gov is just an alias for esgf.org:
>>>>>>>>
>>>>>>>> $ git remote -v
>>>>>>>> origin git at esg-repo.llnl.gov:esgf-security.git (fetch)
>>>>>>>> origin git at esg-repo.llnl.gov:esgf-security.git (push)
>>>>>>>>
>>>>>>>> The HttpClient code has timeouts in it now so upgrading should
>>>>>>>> help with SOAP calls. Our production node still has a fairly old
>>>>>>>> version of esgf-security but there will be an upgrade really
>>>>>>>> soon. It's great I've got an even bigger reason to upgrade!
>>>>>>>>
>>>>>>>> See
>>>>>>>> esgf-security.git/src/java/main/esg/security/common/SOAPServiceClient.java
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Stephen.
>>>>>>>>
>>>>>>>> ---
>>>>>>>> Stephen Pascoe +44 (0)1235 445980
>>>>>>>> Centre of Environmental Data Archival
>>>>>>>> STFC Rutherford Appleton Laboratory, Harwell Oxford, Didcot OX11
>>>>>>>> 0QX, UK
>>>>>>>>
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: go-essp-tech-bounces at ucar.edu
>>>>>>>> [mailto:go-essp-tech-bounces at ucar.edu] On Behalf Of Cinquini, Luca
>>>>>>>> (3880)
>>>>>>>> Sent: 17 February 2012 19:53
>>>>>>>> To: Nathan Wilhelmi
>>>>>>>> Cc: go-essp-tech at ucar.edu
>>>>>>>> Subject: Re: [Go-essp-tech] TDS Authorization issues
>>>>>>>>
>>>>>>>> Hi Nate,
>>>>>>>> it should be esgf-security.git. But again, my advice is to
>>>>>>>> wait till you can install the same software as al the other CMIP5
>>>>>>>> data nodes - it should be early next week.
>>>>>>>> thanks, L
>>>>>>>>
>>>>>>>> On Feb 17, 2012, at 11:14 AM, Nathan Wilhelmi wrote:
>>>>>>>>
>>>>>>>>> Hi Luca,
>>>>>>>>>
>>>>>>>>> Is esg-security available? esg-security.git doesn't seem to be
>>>>>>>>> available?
>>>>>>>>>
>>>>>>>>> Thanks!
>>>>>>>>> -Nate
>>>>>>>>>
>>>>>>>>> On 2/17/2012 11:04 AM, Cinquini, Luca (3880) wrote:
>>>>>>>>>> Hi Nate,
>>>>>>>>>> that is some seriously old code you are using...
>>>>>>>>>>
>>>>>>>>>> The ORP is available from:
>>>>>>>>>>
>>>>>>>>>> git clone git at esgf.org:esg-orp.git
>>>>>>>>>>
>>>>>>>>>> but it looks like the version you are using is the very first
>>>>>>>>>> tag there,
>>>>>>>>>> probably more than one year old.
>>>>>>>>>>
>>>>>>>>>> The esg-saml library does not even exist any more, it's been long
>>>>>>>>>> replaced by esgf-security.
>>>>>>>>>>
>>>>>>>>>> My suggestion would be to wait till next week when we will roll
>>>>>>>>>> out a
>>>>>>>>>> data-node only installation, for use by all CMIP5 data nodes. That
>>>>>>>>>> should allow you to upgrade the UCAR TDS as a data-node only.
>>>>>>>>>>
>>>>>>>>>> thanks, Luca
>>>>>>>>>>
>>>>>>>>>> On Feb 17, 2012, at 10:46 AM, Nathan Wilhelmi wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi All,
>>>>>>>>>>>
>>>>>>>>>>> Version # mistake.
>>>>>>>>>>>
>>>>>>>>>>> We are looking for:
>>>>>>>>>>>
>>>>>>>>>>> esg-node.1.0.0.4.jar
>>>>>>>>>>> esg-node-common.1.0.0.4.jar
>>>>>>>>>>> esg-node-filters.1.0.0.4.jar
>>>>>>>>>>> esg-orp.1.1.2.2.jar
>>>>>>>>>>> esg-saml.1.2.1.jar
>>>>>>>>>>>
>>>>>>>>>>> Thanks!
>>>>>>>>>>> -Nate
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2/17/2012 10:38 AM, Nathan Wilhelmi wrote:
>>>>>>>>>>>> Hi All,
>>>>>>>>>>>>
>>>>>>>>>>>> We upgraded our TDS to the newer no caching version this week.
>>>>>>>>>>>> This
>>>>>>>>>>>> version uses much less RAM and is more responsive. However
>>>>>>>>>>>> this appears
>>>>>>>>>>>> to have uncovered some issues with the authorization related node
>>>>>>>>>>>> components. At this point our TDS will only authorize datasets
>>>>>>>>>>>> for a
>>>>>>>>>>>> short time after a restart. Our TDS will authorize reliably if
>>>>>>>>>>>> only 1
>>>>>>>>>>>> thread is enabled in Tomcat, but this isn't really a workable
>>>>>>>>>>>> solution.
>>>>>>>>>>>>
>>>>>>>>>>>> After some debugging and code review we believe the problem
>>>>>>>>>>>> lies with
>>>>>>>>>>>> HttpClient usage patterns.
>>>>>>>>>>>>
>>>>>>>>>>>> To expedite getting our TDS back online and debugging the
>>>>>>>>>>>> problem we are
>>>>>>>>>>>> trying to get a hold of the source for the node components we
>>>>>>>>>>>> are using.
>>>>>>>>>>>> Where can we get the source for the following jars?
>>>>>>>>>>>>
>>>>>>>>>>>> esg-node.1.0.0.4.jar
>>>>>>>>>>>> esg-node-common.1.0.0.4.jar
>>>>>>>>>>>> esg-node-filters.1.0.0.4.jar
>>>>>>>>>>>> esg-orp.1.0.0.4.jar
>>>>>>>>>>>> esg-saml.1.0.0.4.jar
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks!
>>>>>>>>>>>> -Nate
>>>>>>>>>>>>
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> GO-ESSP-TECH mailing list
>>>>>>>>>>>> GO-ESSP-TECH at ucar.edu<mailto:GO-ESSP-TECH at ucar.edu>
>>>>>>>>>>>> http://mailman.ucar.edu/mailman/listinfo/go-essp-tech
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> GO-ESSP-TECH mailing list
>>>>>>>>>>> GO-ESSP-TECH at ucar.edu<mailto:GO-ESSP-TECH at ucar.edu>
>>>>>>>>>>> http://mailman.ucar.edu/mailman/listinfo/go-essp-tech
>>>>>>>> _______________________________________________
>>>>>>>> GO-ESSP-TECH mailing list
>>>>>>>> GO-ESSP-TECH at ucar.edu
>>>>>>>> http://mailman.ucar.edu/mailman/listinfo/go-essp-tech
>>>>>>>> --
>>>>>>>> Scanned by iCritical.
>>>>>>> --
>>>>>>> Scanned by iCritical.
>>>>> _______________________________________________
>>>>> GO-ESSP-TECH mailing list
>>>>> GO-ESSP-TECH at ucar.edu
>>>>> http://mailman.ucar.edu/mailman/listinfo/go-essp-tech
>>
>> --
>> Estanislao Gonzalez
>>
>> Max-Planck-Institut für Meteorologie (MPI-M)
>> Deutsches Klimarechenzentrum (DKRZ) - German Climate Computing Centre
>> Room 108 - Bundesstrasse 45a, D-20146 Hamburg, Germany
>>
>> Phone: +49 (40) 46 00 94-126
>> E-Mail: gonzalez at dkrz.de
>>
>
--
Estanislao Gonzalez
Max-Planck-Institut für Meteorologie (MPI-M)
Deutsches Klimarechenzentrum (DKRZ) - German Climate Computing Centre
Room 108 - Bundesstrasse 45a, D-20146 Hamburg, Germany
Phone: +49 (40) 46 00 94-126
E-Mail: gonzalez at dkrz.de
More information about the GO-ESSP-TECH
mailing list