oracle / opengrok

OpenGrok is a fast and usable source code search and cross reference engine, written in Java
http://oracle.github.io/opengrok/
Other
4.34k stars 745 forks source link

share LDAP server pools between authorization plugins #3361

Open vladak opened 3 years ago

vladak commented 3 years ago

Another post-mortem induced issue: Whenever a LDAP lookup times out (say with 3 second timeout) I can see the overall request taking disproportionate time (tens/hundreds of seconds), likely depending on the number of projects/groups in the setup (for 40ish groups and 3 seconds timer there was a latency of 117 seconds which is almost the product. This is because the authorization stack is based almost solely on project groups). This happens even with a failover in place.

vladak commented 3 years ago

This could be related to #3360 - the NPE happened when handling NamingException in lookup on lines 405-406 so it could not get to perform the failover:


404          } catch (NamingException ex) {
405              LOGGER.log(Level.SEVERE, String.format("An arbitrary LDAP error occurred on server %s " +
406                      "when searching for '%s'", server, getSearchDescription(dn, filter, attributes)), ex);
407              closeActualServer();
408              actualServer = getNextServer();
409              return lookup(dn, filter, attributes, mapper, fail + 1);
410          } finally {
vladak commented 3 years ago

Each authorization plugin extends AbstractLdapPlugin which has its own instance of LdapFacade (constructed in AbstractLdapPlugin#load()) that performs the failover. This means that each plugin will have to failover individually.

vladak commented 3 years ago

The consequence is that there is no connection sharing between plugins. There are as many connections to the same LDAP server (assuming normal circumstances) as there are types of plugins in the stack. I am not entirely sure this is a good or bad thing. From the perspective of someone running a LDAP server probably the latter.

vladak commented 3 years ago

The solution I have in mind is to move most logic/data from the LdapFacade to new LdapPool class, e.g. it would hold a list of servers, timeouts, webhooks, search controls, search base etc. There will be as many instances of LdapFacade as there are configuration files (do not want to complicate the implementation with sharing down to single server, assuming the server pools are usually disjoint w.r.t. servers), each will be retrieved via static method LdapFacade.getFacade(String configurationPath) and LdapFacade will have Map of the instances. This will have impact on LdapFacade.close() called from AbstractLdapPlugin#unload() and generally on the AuthorizationFramework reload.

tulinkry commented 3 years ago

Or a new plugin LdapServerPlugin, marked as requisite and populating a request attribute with LdapFacade which initializes just once. This wouldn't require a change in the core.

vladak commented 3 years ago

Or a new plugin LdapServerPlugin, marked as requisite and populating a request attribute with LdapFacade which initializes just once. This wouldn't require a change in the core.

Interesting idea :-)

vladak commented 1 month ago

Another alternative class to use for reference counting might be the abstract ReferenceManager from Lucene.