You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "Artem Abeleshev (Jira)" <ji...@apache.org> on 2022/07/04 05:46:00 UTC
[jira] [Updated] (SOLR-16282) Improve custom actions support of CoreAdminHandler
[ https://issues.apache.org/jira/browse/SOLR-16282?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Artem Abeleshev updated SOLR-16282:
-----------------------------------
Description:
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 action 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 code block 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:
```java
...
final String action = req.getParams().get(ACTION, STATUS.toString()).toLowerCase(Locale.ROOT);
CoreAdminOp op = opMap.get(action);
if (op == null)
{ op = getCustomOperation(action); }
...
```
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
was:
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 action 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)__
```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 {
...
}
}
...
}
```
As we can see, the call to the `handleRequestBody` is just a call to the custom code block 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:
```java
...
final String action = req.getParams().get(ACTION, STATUS.toString()).toLowerCase(Locale.ROOT);
CoreAdminOp op = opMap.get(action);
if (op == null) {
op = getCustomOperation(action);
}
...
```
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
> Improve custom actions support of CoreAdminHandler
> --------------------------------------------------
>
> Key: SOLR-16282
> URL: https://issues.apache.org/jira/browse/SOLR-16282
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Affects Versions: main (10.0)
> Reporter: Artem Abeleshev
> Priority: Minor
>
> 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 action 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 code block 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:
> ```java
> ...
> final String action = req.getParams().get(ACTION, STATUS.toString()).toLowerCase(Locale.ROOT);
> CoreAdminOp op = opMap.get(action);
> if (op == null)
> { op = getCustomOperation(action); }
> ...
> ```
> 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-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org