[
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:
{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
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)_
{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
> 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:
>
> {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: [email protected]
For additional commands, e-mail: [email protected]
