Copilot commented on code in PR #17494:
URL: https://github.com/apache/pinot/pull/17494#discussion_r2755794798
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -672,71 +888,83 @@ public Map<String, Object> getCronSchedulerJobDetails(
@ApiOperation("Schedule tasks and return a map from task type to task name
scheduled. If task type is missing, "
+ "schedules all tasks. If table name is missing, schedules tasks for
all tables in the database. If database "
+ "is missing in headers, uses default.")
- @Nullable
- public Map<String, String> scheduleTasks(
+ @ManagedAsync
+ public void scheduleTasks(
@ApiParam(value = "Task type. If missing, schedules all tasks.")
@QueryParam("taskType") @Nullable
String taskType,
@ApiParam(value = "Table name (with type suffix). If missing, schedules
tasks for all tables in the database.")
@QueryParam("tableName") @Nullable String tableName,
@ApiParam(value = "Minion Instance tag to schedule the task explicitly
on") @QueryParam("minionInstanceTag")
- @Nullable String minionInstanceTag, @Context HttpHeaders headers) {
- String database = headers != null ? headers.getHeaderString(DATABASE) :
DEFAULT_DATABASE;
- Map<String, String> response = new HashMap<>();
- List<String> generationErrors = new ArrayList<>();
- List<String> schedulingErrors = new ArrayList<>();
- TaskSchedulingContext context = new TaskSchedulingContext()
- .setTriggeredBy(CommonConstants.TaskTriggers.MANUAL_TRIGGER.name())
- .setMinionInstanceTag(minionInstanceTag)
- .setLeader(false);
- if (taskType != null) {
- context.setTasksToSchedule(Collections.singleton(taskType));
- }
- if (tableName != null) {
-
context.setTablesToSchedule(Collections.singleton(DatabaseUtils.translateTableName(tableName,
headers)));
- } else {
- context.setDatabasesToSchedule(Collections.singleton(database));
- }
- Map<String, TaskSchedulingInfo> allTaskInfos =
_pinotTaskManager.scheduleTasks(context);
- allTaskInfos.forEach((key, value) -> {
- if (value.getScheduledTaskNames() != null) {
- response.put(key, String.join(",", value.getScheduledTaskNames()));
+ @Nullable String minionInstanceTag, @Context HttpHeaders headers,
@Suspended AsyncResponse asyncResponse) {
+ try {
+ String database = headers != null ? headers.getHeaderString(DATABASE) :
DEFAULT_DATABASE;
+ Map<String, String> response = new HashMap<>();
+ List<String> generationErrors = new ArrayList<>();
+ List<String> schedulingErrors = new ArrayList<>();
+ TaskSchedulingContext context = new TaskSchedulingContext()
+ .setTriggeredBy(CommonConstants.TaskTriggers.MANUAL_TRIGGER.name())
+ .setMinionInstanceTag(minionInstanceTag)
+ .setLeader(false);
+ if (taskType != null) {
+ context.setTasksToSchedule(Collections.singleton(taskType));
}
- generationErrors.addAll(value.getGenerationErrors());
- schedulingErrors.addAll(value.getSchedulingErrors());
- });
- response.put(GENERATION_ERRORS_KEY, String.join(",", generationErrors));
- response.put(SCHEDULING_ERRORS_KEY, String.join(",", schedulingErrors));
- return response;
+ if (tableName != null) {
+
context.setTablesToSchedule(Collections.singleton(DatabaseUtils.translateTableName(tableName,
headers)));
+ } else {
+ context.setDatabasesToSchedule(Collections.singleton(database));
+ }
+ Map<String, TaskSchedulingInfo> allTaskInfos =
_pinotTaskManager.scheduleTasks(context);
+ allTaskInfos.forEach((key, value) -> {
+ if (value.getScheduledTaskNames() != null) {
+ response.put(key, String.join(",", value.getScheduledTaskNames()));
+ }
+ generationErrors.addAll(value.getGenerationErrors());
+ schedulingErrors.addAll(value.getSchedulingErrors());
+ });
+ response.put(GENERATION_ERRORS_KEY, String.join(",", generationErrors));
+ response.put(SCHEDULING_ERRORS_KEY, String.join(",", schedulingErrors));
+ asyncResponse.resume(response);
+ } catch (Exception e) {
+ asyncResponse.resume(new ControllerApplicationException(LOGGER,
+ String.format("Failed to schedule tasks due to error: %s",
ExceptionUtils.getStackTrace(e)),
+ Response.Status.INTERNAL_SERVER_ERROR, e));
+ }
}
@POST
- @ManagedAsync
@Produces(MediaType.APPLICATION_JSON)
@Path("/tasks/execute")
@Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.EXECUTE_TASK)
@Authenticate(AccessType.CREATE)
@ApiOperation("Execute a task on minion")
+ @ManagedAsync
public void executeAdhocTask(AdhocTaskConfig adhocTaskConfig, @Suspended
AsyncResponse asyncResponse,
@Context Request requestContext) {
try {
-
asyncResponse.resume(_pinotTaskManager.createTask(adhocTaskConfig.getTaskType(),
adhocTaskConfig.getTableName(),
- adhocTaskConfig.getTaskName(), adhocTaskConfig.getTaskConfigs()));
- } catch (TableNotFoundException e) {
- throw new ControllerApplicationException(LOGGER, "Failed to find table:
" + adhocTaskConfig.getTableName(),
- Response.Status.NOT_FOUND, e);
- } catch (TaskAlreadyExistsException e) {
- throw new ControllerApplicationException(LOGGER, "Task already exists: "
+ adhocTaskConfig.getTaskName(),
- Response.Status.CONFLICT, e);
- } catch (UnknownTaskTypeException e) {
- throw new ControllerApplicationException(LOGGER, "Unknown task type: " +
adhocTaskConfig.getTaskType(),
- Response.Status.NOT_FOUND, e);
- } catch (NoTaskScheduledException e) {
- throw new ControllerApplicationException(LOGGER,
- "No task is generated for table: " + adhocTaskConfig.getTableName()
+ ", with task type: "
- + adhocTaskConfig.getTaskType(), Response.Status.BAD_REQUEST, e);
+ try {
+ asyncResponse.resume(
+ _pinotTaskManager.createTask(adhocTaskConfig.getTaskType(),
adhocTaskConfig.getTableName(),
Review Comment:
The `executeAdhocTask` method has nested try-catch blocks that reduce
readability. The outer try-catch at line 943 catches all exceptions and calls
`asyncResponse.resume(e)`, while the inner try-catch at line 944 catches
specific exceptions and throws `ControllerApplicationException`. Since the
outer catch will handle the exceptions thrown by the inner block, consider
consolidating these into a single try-catch structure or removing the outer
try-catch if the inner exception handling is sufficient.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
