solr icon indicating copy to clipboard operation
solr copied to clipboard

SOLR-16282: Update core admin handler to use pluggable custom actions

Open ijioio opened this issue 3 years ago • 7 comments

https://issues.apache.org/jira/browse/SOLR-16282

Description

At the moment implementing custom actions for CoreAdminHandler implies subsclassing of the CoreAdminHandler and overriding handleCustomAction method. This has numerous problems. First, it seems a bit too much for the simple concept of implementing custom actions and quite quirky (out of the way Solr usually do things). The second thing is, current custom actions implementation is just a block of code to be called, so it is not integrated into the standard actions workflow and doesn't use async facility. Third, it has the following problem: let's say, one developer created CoreAdminHandler with action A, and another developer created CoreAdminHandler with action B. If the user want to use both actions A and B user will get a problem, as user can override only one handler, i.e no handler "chaining" is supported.

Solution

Add support for pluggable custom actions that will be managed the same way as standard ones. No need to subclass CoreAdminHandler at all. Just implement CoreAdminOp interface and define actions within solr.xml.

Example:

com.ijioio.solr.action.FooAction

public class FooAction implements CoreAdminOp {

  @Override
  public void execute(CallInfo it) throws Exception {
	  
      NamedList<Object> details = new SimpleOrderedMap<>();
      
      details.add("startTime", Instant.now().toString());
      
      // do some stuff
      
      details.add("status", "success");
      details.add("endTime", Instant.now().toString());

      it.rsp.addResponse(details);
  }
}

solr.xml

   ...
  <coreAdminHandlerActions>
    <str name="foo">com.ijioio.solr.action.FooAction</str>
    <str name="bar">com.ijioio.solr.action.BarAction</str>
  </coreAdminHandlerActions>
  ...

Now it possible to call these action by sending requests.

Sync request

Request: http://localhost:8983/solr/admin/cores?action=foo Response:

{
    "responseHeader": {
        "status": 0,
        "QTime": 8023
    },
    "response": {
        "startTime": "2022-07-05T03:35:57.365294500Z",
        "status": "success",
        "endTime": "2022-07-05T03:35:57.365547900Z"
    }
}

Async request

Request: http://localhost:8983/solr/admin/cores?action=foo&async=1000 Response:

{
    "responseHeader": {
        "status": 0,
        "QTime": 4020
    }
}

Request: http://localhost:8983/solr/admin/cores?action=requeststatus&requestid=1000 Response:

{
    "responseHeader": {
        "status": 0,
        "QTime": 2308
    },
    "STATUS": "completed",
    "msg": "TaskId: 1000 webapp=null path=/admin/cores params={async=1000&action=foo} status=0 QTime=4020",
    "response": {
        "startTime": "2022-07-05T03:36:21.910596800Z",
        "status": "success",
        "endTime": "2022-07-05T03:36:21.910596800Z"
    }
}

Checklist

Please review the following and check all that apply:

  • [x] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • [x] I have created a Jira issue and added the issue ID to my pull request title.
  • [x] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • [x] I have developed this patch against the main branch.
  • [ ] I have run ./gradlew check.
  • [x] I have added tests for my changes.
  • [x] I have added documentation for the Reference Guide

ijioio avatar Jul 05 '22 07:07 ijioio

Hello @ijioio!

Thanks for the detailed notes both on the https://issues.apache.org/jira/browse/SOLR-16282 JIRA and the pull request here, they really help with understanding of the existing code and the proposed changes to it!

I like the idea of custom actions being pluggable rather than CoreAdminHandler needing to be sub-classed.

It's a bit subjective of course but the solr.xml needing to be changed, that seems a bit of a burden, both operationally and in terms of code complexity? I wonder if there might be some way to dynamically discover actions e.g. if the action was a fully qualified class path and CoreAdminHandler had access to SolrResourceLoader then could SolrResourceLoader.newInstance instantiate the action on demand?

http://localhost:8983/solr/admin/cores?action=com.ijioio.solr.action.FooAction

From a use point of view the action being a class path seems not very nice though. We could make up some convention where the pluggable class always starts with (say) CustomCoreAdminOp and where the action follows after that (say) CustomCoreAdminOpFoo but I don't know if going from foo action to CustomCoreAdminOpFoo class name would be enough to actually instantiate an object. Plus there could be two classes with the same name in different name spaces. Hmm.

cpoerschke avatar Jul 11 '22 18:07 cpoerschke

Hi @cpoerschke! Thanks for the comment!

It's a bit subjective of course but the solr.xml needing to be changed, that seems a bit of a burden, both operationally and in terms of code complexity? I wonder if there might be some way to dynamically discover actions e.g. if the action was a fully qualified class path and CoreAdminHandler had access to SolrResourceLoader then could SolrResourceLoader.newInstance instantiate the action on demand?

Yeah, that would be nice to not bother with solr.xml, but at the moment I doesn't see nice workaround.

Pluggable nature usually implies classes should be defined somewhere (*.xml, manifest.mf, etc). On system start, plugins files are examined for plugin extention points, and some global registry is populated with the relevant entries (let's not take into account some plugin frameworks that can do runtime loading/unloading , like OSGi). So we can say we applied exactly the same idea above. Solr can be assumed as "pluggable" to some extent, as custom things can be redefined in configuration (solr.xml or solrconfig.xml), but Solr still is not fully qualified pluggable (no plugin extension points, no separation/isolation of classloaders).

The other way to do discovery, is dynamic class descovery by scanning the classpath. From one side it looks very powerfull, you can use annotations for marking classes, etc. But at the same time it is quite heavy and implies unnecassary class loading.

I am still examing different parts of Solr, so maybe I have missed something, and there are already some examples of dynamic discovery of classes. If you know of some examples, give me a note please.

From a use point of view the action being a class path seems not very nice though. We could make up some convention where the pluggable class always starts with (say) CustomCoreAdminOp and where the action follows after that (say) CustomCoreAdminOpFoo but I don't know if going from foo action to CustomCoreAdminOpFoo class name would be enough to actually instantiate an object. Plus there could be two classes with the same name in different name spaces. Hmm.

It is possible to go the way you suggested, we can make an interface implying the action name definition:

public interface CustomCoreAdminOp extends CoreAdminOp {

	public String getName();
}

and even provide some way to override/alias the action name. But I am agree, pushing the full qualified class name is looks too much.

Another idea is, first users need to execute some register action request, i.e.

http://localhost:8983/solr/admin/cores?action=registeraction&class=com.ijioio.solr.action.FooAction&name=foo

But it implies always running this register action on all the shards, at least for cases when your action is a part of collection related logic :( Feels quite tricky and not transparent.

ijioio avatar Jul 12 '22 06:07 ijioio

... we can make an interface implying the action name definition ...

I wonder if with such an interface the CoreAdminHandler subclassing could be more favourable after all e.g.

+ @Deprecated
  protected void handleCustomAction(SolrQueryRequest req, SolrQueryResponse rsp) {
    ...
  }
+
+ protected Collection<CustomCoreAdminOp> getCustomCoreAdminOps() {
+   return Collections.emptyList();
+ }

i.e. no solr.xml change and also no mention of the hypothetical foo and bar actions in solr.xml or solrconfig.xml i.e. if first there's only a foo action and later bar is added then there's no need to change any configuration at that later point in time, only the initial custom CoreAdminHandler configuration change is needed.

cpoerschke avatar Jul 13 '22 11:07 cpoerschke

Sorry, I doesn't get the idea ^:) If we create CustomCoreAdminOp interface and add the method getCustomCoreAdminOps() you mentiond above, there is still the question of how to populate the collection of custom actions. Do you mean by subcalssing the CoreAdminHandler and populate it explicitly?

ijioio avatar Jul 14 '22 09:07 ijioio

... Do you mean by subcalssing the CoreAdminHandler and populate it explicitly?

Yes, subclassing and explicit populating in the subclass would be one way e.g. in solrconfig.xml simply this then:

<requestHandler name="/admin" class="com.ijioio.solr.CoreAdminHandler"/>

But if people can subclass then they can implement in whatever way suits them and they might want their class to be configurable e.g. like this:

<requestHandler name="/admin" class="com.ijioio.solr.ConfigurableCoreAdminHandler">
    <str name="foo">com.ijioio.solr.action.FooAction</str>
    <str name="bar">com.ijioio.solr.action.BarAction</str>
</requestHandler>

And following that thought further, if we think that configurable custom use is better than non-configurable use then we could have the in-built CoreAdminHandler support configuration e.g. something like this:

<requestHandler name="/admin" class="org.apache.solr.handler.admin.CoreAdminHandler">
  <lst name="customActions">
    <str name="foo">com.ijioio.solr.action.FooAction</str>
    <str name="bar">com.ijioio.solr.action.BarAction</str>
  </lst>
</requestHandler>

cpoerschke avatar Jul 14 '22 09:07 cpoerschke

Actually based on https://github.com/apache/solr/blob/releases/solr/9.0.0/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java#L121-L127 it seems that a configurable CoreAdminHandler would not work.

Not yet checked if <requestHandler name="/admin" class="com.ijioio.solr.CoreAdminHandler"/> is possible.

cpoerschke avatar Jul 14 '22 09:07 cpoerschke

Not yet checked if <requestHandler name="/admin" class="com.ijioio.solr.CoreAdminHandler"/> is possible.

It is not, you can't redefine admin handler via solrconfig.xml. It is a special global handler of instance scale, not handler of core scale, so only one common instance is possible, it it expected to be redefined via solr.xml.

It is created only once during load() call of CoreContianer and can be overriden by defining adminHandler value in solr.xml:

org.apache.solr.core.CoreContainer

public void load() {
  ...
    coreAdminHandler =
        createHandler(CORES_HANDLER_PATH, cfg.getCoreAdminHandlerClass(), CoreAdminHandler.class);
  ...
}

org.apache.solr.core.SolrXmlConfig

  private static NodeConfig fillSolrSection(
      NodeConfig.NodeConfigBuilder builder, NamedList<Object> nl) {

    for (Map.Entry<String, Object> entry : nl) {
      String name = entry.getKey();
      if (entry.getValue() == null) continue;
      String value = entry.getValue().toString();
      switch (name) {
        case "adminHandler":
          builder.setCoreAdminHandlerClass(value);
          break;
        ...
      }
    }

    return builder.build();
  }

If you try to define it on core level, via path mapping, it will end up calling init() and will fail.

org.apache.solr.handler.admin.CoreAdminHandler

  @Override
  public final void init(NamedList<?> args) {
    throw new SolrException(
        SolrException.ErrorCode.SERVER_ERROR,
        "CoreAdminHandler should not be configured in solrconf.xml\n"
            + "it is a special Handler configured directly by the RequestDispatcher");
  }

That is the reason why I think redefining a set of actions via solr.xml looks more clean than redefining CoreAdminHandler (that in turn will explicitly collect actions) via the same solr.xml. This also solves the following problem: let's say, one developer create CoreAdminHandler with action A, and another developer create CoreAdminHandler with action B. If the user want to use both actions A and B user will get a problem, as user can override only one handler, i.e no handler "chaining" is supported. While actions are just plugs in to default handler and not dependent on each other.

ijioio avatar Jul 14 '22 12:07 ijioio