vigneshio commented on code in PR #4962:
URL: https://github.com/apache/polaris/pull/4962#discussion_r3593359700
##########
runtime/service/src/main/java/org/apache/polaris/service/task/TaskExecutorImpl.java:
##########
@@ -229,35 +230,39 @@ protected void handleTask(
.addKeyValue("taskEntityId", taskEntityId)
.addKeyValue("taskType", task.getTaskType())
.log("Unable to find handler for task type");
- throw new RuntimeException(
+ throw new TaskHandlerNotFoundException(
"Unable to find handler for task type "
+ task.getTaskType()
+ " for task entity id "
+ taskEntityId);
}
TaskHandler handler = handlerOpt.get();
- success = handler.handleTask(task, ctx);
- if (success) {
- LOGGER
- .atInfo()
- .addKeyValue("taskEntityId", taskEntityId)
- .addKeyValue("handlerClass", handler.getClass())
- .log("Task successfully handled");
- metaStoreManager.dropEntityIfExists(
- ctx.getPolarisCallContext(), null, taskEntity, Map.of(), false);
- } else {
+ // Normal return from handleTask means the task work completed
successfully. Set success
+ // before dropEntityIfExists so a later cleanup failure still reports
TASK_SUCCESS=true
+ // (the exception is rethrown for retry/logging, but the attempt is not
a handler failure).
+ handler.handleTask(task, ctx);
+ success = true;
+ LOGGER
+ .atInfo()
+ .addKeyValue("taskEntityId", taskEntityId)
+ .addKeyValue("handlerClass", handler.getClass())
+ .log("Task successfully handled");
+ metaStoreManager.dropEntityIfExists(
+ ctx.getPolarisCallContext(), null, taskEntity, Map.of(), false);
+ } catch (TaskHandlerNotFoundException e) {
+ // success stays false (never set true). Re-throw without the generic
failure log below.
+ throw e;
+ } catch (Exception e) {
+ // Do not force success=false: handleTask may already have completed
(success=true) and the
+ // failure may be from post-completion cleanup (e.g. dropEntityIfExists).
+ if (!success) {
LOGGER
.atWarn()
.addKeyValue("taskEntityId", taskEntityId)
- .addKeyValue("taskEntityName", taskEntity.getName())
+ .addKeyValue("taskEntityName", taskEntity != null ?
taskEntity.getName() : "")
.log("Unable to execute async task");
- throw new RuntimeException(
- "Task handler returned false for task entity id "
- + taskEntityId
- + " (handler: "
- + handler.getClass().getSimpleName()
- + ")");
}
+ throw e;
Review Comment:
@dimas-b no, that log line is gated on success being false, so it only
appears for genuine handleTask failures. If cleanup throws after handleTask
completed successfully, the warning is skipped; the exception propagates
normally and is handled/logged by the retry mechanism.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
