[
https://issues.apache.org/jira/browse/SOLR-16282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17691681#comment-17691681
]
Jason Gerlowski commented on SOLR-16282:
----------------------------------------
OK, I definitely see those benefits and limitations you're pointing to. Thanks
for clarifying: I should've read through the PR before asking, but I usually
assume (wrongly it turns out) that most PR discussion is lower level code
review.
I guess the question I'm ultimately trying to understand and answer is: does it
make sense for Solr to support as many "add this new endpoint to Solr"
extension points as it currently does? It sounds like currently, the answer is
"yes" for the reasons you mentioned. But if we address some of the pain points
underlying that (e.g. making the infrastructure around 'async' requests more
re-usable) maybe we can get to a point (in say, Solr 10) where more
streamlining is possible.
> Improve custom actions support of CoreAdminHandler
> --------------------------------------------------
>
> Key: SOLR-16282
> URL: https://issues.apache.org/jira/browse/SOLR-16282
> Project: Solr
> Issue Type: Improvement
> Affects Versions: main (10.0)
> Reporter: Artem Abeleshev
> Assignee: Christine Poerschke
> Priority: Minor
> Fix For: 9.1, main (10.0)
>
> Time Spent: 7h 10m
> Remaining Estimate: 0h
>
> Original _CoreAdminHandler_
> ({_}org.apache.solr.handler.admin.CoreAdminHandler{_}) has a support of
> custom actions by providing _handleCustomAction_ method. It is intended for
> users who want to implement an additional actions (for example, for some
> instumental or statistical purposes). By default _handleCustomAction_ method
> throws an exception implying user should subclass handler and provide its own
> _handleCustomAction_ method implementation. But there are some structural
> problems.
> If we check how the _CoreAdminHandler_ triggers the _handleCustomAction_
> method we will see that it is always runs in a _sync_ way. Despite the fact
> that _CoreAdminHandler_ has nice support of running the actions in an _async_
> way. Moreover, if user push the custom action request with an _async_
> parameter it will create _TaskObject_ object and will place it to the
> tracking map occupying one slot and will never clean it up:
> _org.apache.solr.handler.admin.CoreAdminHandler.handleRequestBody(SolrQueryRequest,
> SolrQueryResponse)_
> {code:java}
> @Override
> public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp)
> throws Exception {
> ...
> final String taskId = req.getParams().get(CommonAdminParams.ASYNC);
> final TaskObject taskObject = new TaskObject(taskId);
> if (taskId != null) {
> ...
> addTask(RUNNING, taskObject);
> }
> final String action = req.getParams().get(ACTION,
> STATUS.toString()).toLowerCase(Locale.ROOT);
> CoreAdminOperation op = opMap.get(action);
> if (op == null) {
> handleCustomAction(req, rsp);
> return;
> }
>
> final CallInfo callInfo = new CallInfo(this, req, rsp, op);
> ...
> if (taskId == null) {
> callInfo.call();
> } else {
> try {
> ...
> parallelExecutor.execute(
> () -> {
> boolean exceptionCaught = false;
> try {
> callInfo.call();
> taskObject.setRspObject(callInfo.rsp);
> taskObject.setOperationRspObject(callInfo.rsp);
> } catch (Exception e) {
> exceptionCaught = true;
> taskObject.setRspObjectFromException(e);
> } finally {
> removeTask("running", taskObject.taskId);
> if (exceptionCaught) {
> addTask("failed", taskObject, true);
> } else {
> addTask("completed", taskObject, true);
> }
> }
> });
> } finally {
> ...
> }
> }
> ...
> } {code}
> As we can see, the call to the _handleRequestBody_ is just a call to the
> custom block of the code that is not weaved nicely to the overall worflow. I
> suggest to update the logic to not just call custom block of the code, but
> instead to force it to provide a _CoreAdminOp_ instance, that would be used
> in the further execution as a regular operation extracterd from the
> {_}opMap{_}. Like this:
>
> {code:java}
> ...
> final String action = req.getParams().get(ACTION,
> STATUS.toString()).toLowerCase(Locale.ROOT);
> CoreAdminOp op = opMap.get(action);
> if (op == null) {
> op = getCustomOperation(action);
> }
> ...
> {code}
>
> This way the custom actions can be easily integrated in the general worflow
> with minimal efforts. In result we will get:
> - support of an async custom actions
> - using of the standard _CoreAdminOp_ and _CallInfo_ structures
> - more clean code
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
