Improve task retrieval APIs on Overlord by surekhasaharan · Pull Request #5801 · apache/druid

surekhasaharan · 2018-05-24T18:02:33Z

Add the new tasks api in overlordResource

It takes 5 optional query params
* state(pending/running/waiting/complete)
* dataSource
* interval (applies to completed tasks)
* max(applies to completed tasks)
* type (task type)

If all params are null, the api returns all the tasks.
This api improves the performance of retrieving all tasks as well as improves existing API's 
`runningTasks` `completeTasks` `waitingTasks` `pendingTasks` by retrieving the tasks data from 
 database once instead of making separate query per task to do authorization checks.

It takes 4 optional query params * state(pending/running/waiting/compelte) * dataSource * interval (applies to completed tasks) * maxCompletedTasks (applies to completed tasks) If all params are null, the api returns all the tasks

* divide active tasks into waiting, pending or running * Add more unit tests

…t-api

jihoonson · 2018-05-25T01:31:45Z

 public enum TaskState
 {
  RUNNING,
+  WAITING,


Would you add descriptions for each state to here?

And I wonder why these new states are needed in this PR. Would you share your thoughts more about this?

This is an incompatible change. This PR should be tagged with Release notes and a note for rolling update should be added to the release notes if this change is needed.

The tasks API returns all tasks if no state query param is passed, so the user of this API wants to know statusCode for each of the tasks. Current statusCode consist of running, success and failed. We need to also send appropriate statusCode for waiting and pending. That's why added those here. We can sure mark this PR as Release notes if you agree to this change.

IMO, TaskState is not the right place to add these things. That's because TaskState's purpose is to model task status as stored in the database (see TaskStorage.insert and TaskStorage.setStatus), where there is no concept of "waiting" or "pending" (those are both stored as "running"). Splitting "running" into waiting/pending/really-really-running is more of a task runner concept.

So I think a new class would make more sense, geared towards modeling the runtime state of a task. Maybe RuntimeTaskState. And that one shouldn't be persisted into the database.

Please write javadocs that make the distinction clear, too.

If I make another RuntimeTaskState do you suggest that to contain all the states : running pending waiting complete success failed or just subclass of running i.e. running waiting pending. The reason I ask is TaskStatusPlus object contains a single property called statusCode, which is the TaskState and UI uses this property to fill status. Should we create two separate status codes or combine everything in one. I prefer latter, i.e. return one status code.

I was thinking the runtime task status code should be its own thing (not a subclass of task state).

okay, i added RunnerTaskState and now TaskStatusPlus will return 2 status codes for each task statusCode and runnerStatusCode.

jihoonson · 2018-05-25T01:32:02Z

  SUCCESS,
-  FAILED;
+  FAILED,
+  UNKNOWN;


What does UNKNOWN state mean?

So instead of passing null for status, I added this UNKNOWN state, when we don't know the state of a task.

jihoonson · 2018-05-25T01:47:34Z

+    if (state != null) {
+      switch (StringUtils.toUpperCase(state)) {
+        case "PENDING":
+          return workItemsResponse(


There are already endpoints to get pending, waiting, running, or complete tasks. Please share the code path rather than duplicating.

Yes, the idea is to potentially call this tasks api with appropriate state query param from the existing endpoints to get pending, waiting, running, complete tasks. I did not do that change yet as this api is still being tested in nightly, will do that once this api passes those tests.

jihoonson · 2018-05-25T01:49:26Z

  }

-  private static class WaitingTask extends TaskRunnerWorkItem
+  private static class AnyTaskBuilder extends TaskRunnerWorkItem


This class is not a builder. Please change to a more appropriate name.

yeah, i was hesitant to name it this way, i added one method withTaskState which is kind of builder.

It's only a builder if it has a build() method, or something like that, that creates an object of a different type. This is more like a normal non-builder class.

IMO, given that this is not really a builder class, the withTaskState method is misleadingly named. Generally, on a non-builder class, you'd expect withX to make a copy with X changed. This is more of a setter, since it mutates the object. So a better name would be setTaskState. (It can still return this, if you want.)

Or, you could design the class to be immutable. In that case, make the field final, call the method withTaskState, and have it return a copy with a different state.

My personal preference would be to do an immutable class, because immutable classes are easier to reason about (you know they won't mutate on you after you get one).

jihoonson · 2018-05-25T01:50:05Z

+          task.getType(),
+          SettableFuture.create(),
+          task.getDataSource(),
+          TaskState.PENDING


Why is the state fixed to pending?

Initially, the state is set to PENDING as a placeholder for all active tasks, later in the method, we filter out the RUNNING and WAITING tasks from this list, so the remaining tasks would be actually in PENDING state.

jihoonson · 2018-05-25T01:50:37Z

+    final List<Task> allActiveTasks = taskStorageQueryAdapter.getActiveTasks();
+    final List<TaskRunnerWorkItem> allTasks = Lists.newArrayList();
+    for (final Task task : allActiveTasks) {
+      allTasks.add(new AnyTaskBuilder(


Druid's convention is

allTasks.add( new AnyTaskBuilder( task.getId(), task.getType(), SettableFuture.create(), task.getDataSource(), TaskState.PENDING ) );

gianm · 2018-05-25T02:00:57Z

+  )
+  {
+
+    if (state != null) {


Is it possible to structure this instead like the following? It would make things clearer, I think, and eliminate the extra code for the "null state" case.

List<Something> tasksIAmGoingToReturn; if (state == null || "pending".equals("state")) { thePendingTasks = someCodeThatGetsPendingTasks(); tasksIAmGoingToReturn.addAll(thePendingTasks); } if (state == null || "waiting".equals("state")) { theWaitingTasks = someCodeThatGetsWaitingTasks(); tasksIAmGoingToReturn.addAll(theWaitingTasks); } if (state == null || "running".equals("state")) { theRunningTasks = someCodeThatGetsRunningTasks(); tasksIAmGoingToReturn.addAll(theRunningTasks); } if (state == null || "complete".equals("state")) { theCompleteTasks = someCodeThatGetsCompleteTasks(); tasksIAmGoingToReturn.addAll(theCompleteTasks); } return tasksIAmGoingToReturn;

In this world, presumably the existing /runningTasks endpoint could be implemented like:

// call the same function as above theRunningTasks = someCodeThatGetsRunningTasks(); return theRunningTasks;

gianm · 2018-05-25T17:52:08Z

 public enum TaskState
 {
  RUNNING,
+  WAITING,


IMO, TaskState is not the right place to add these things. That's because TaskState's purpose is to model task status as stored in the database (see TaskStorage.insert and TaskStorage.setStatus), where there is no concept of "waiting" or "pending" (those are both stored as "running"). Splitting "running" into waiting/pending/really-really-running is more of a task runner concept.

So I think a new class would make more sense, geared towards modeling the runtime state of a task. Maybe RuntimeTaskState. And that one shouldn't be persisted into the database.

Please write javadocs that make the distinction clear, too.

gianm · 2018-05-25T18:01:45Z

  }

-  private static class WaitingTask extends TaskRunnerWorkItem
+  private static class AnyTaskBuilder extends TaskRunnerWorkItem


It's only a builder if it has a build() method, or something like that, that creates an object of a different type. This is more like a normal non-builder class.

IMO, given that this is not really a builder class, the withTaskState method is misleadingly named. Generally, on a non-builder class, you'd expect withX to make a copy with X changed. This is more of a setter, since it mutates the object. So a better name would be setTaskState. (It can still return this, if you want.)

Or, you could design the class to be immutable. In that case, make the field final, call the method withTaskState, and have it return a copy with a different state.

My personal preference would be to do an immutable class, because immutable classes are easier to reason about (you know they won't mutate on you after you get one).

gianm · 2018-05-25T18:11:11Z

+  {
+    Function<TaskStatusPlus, Iterable<ResourceAction>> raGenerator = taskStatusPlus -> {
+      final String taskId = taskStatusPlus.getId();
+      final Optional<Task> optionalTask = taskStorageQueryAdapter.getTask(taskId);


This is still doing taskStorageQueryAdapter.getTask(taskId) for every single task. Let's try to structure the code to avoid this somehow. Possibly by retrieving dataSource info upfront for all relevant tasks, in one SQL call, and then caching it.

gianm · 2018-05-25T18:16:26Z

+    final List<TaskStatusPlus> completeTasks = Lists.newArrayList(Iterables.transform(
+        recentlyFinishedTasks,
+        status -> {
+          final Pair<DateTime, String> pair = taskStorageQueryAdapter.getCreatedDateAndDataSource(status.getId());


This is still doing taskStorageQueryAdapter.getCreatedDateAndDataSource for every single complete task. Let's try to structure the code to avoid this somehow. Possibly by adding a method to taskStorageQueryAdapter that returns this info for all complete tasks at once.

Added new class to capture task info for caching Other refactoring

…t-api

so it can be accessed within server And other related classes like TaskState and TaskStatusPlus are in api

…t-api

This is to mainly capture the task type from payload

This reverts commit 2a527a0.

…t-api

* Add RunnerTaskState

…t-api

* Fix the createdTime and queueInsertionTime for running/active tasks * Clean up tests

…t-api

gianm · 2018-06-11T14:38:41Z

+  WAITING,
+  PENDING,
+  RUNNING,
+  NONE; // is used for a completed task


Might "NONE" also be used for tasks that the runner doesn't know about yet? This could be the case during bootstrapping, i.e. in the time between when a overlord becomes the new leader and when its runner has actually been fed all the tasks that need to run.

The tasks which are not in TaskRunner.getKnownTasks and not COMPLETE are marked in WAITING state. I am using this NONE state for completed tasks to keep common datatype for all returned tasks.

Got it. That sounds fair.

gianm · 2018-06-11T14:50:32Z

+ */
+public class TaskInfo
+{
+  private String id;


These fields should be final, because in general, any fields that don't need to be modified should be final. It helps improve code readability (no need to wonder whether the field values change over time) and reduces bugs (avoids accidentally assuming fields are constant when they might actually be modified).

If all fields are final, then instances of the class become immutable, which are nice for a bunch of reasons: https://hackernoon.com/5-benefits-of-immutable-objects-worth-considering-for-your-next-project-f98e7e85b6ac

thanks, nice article.

gianm · 2018-06-11T16:02:24Z

+    this.dataSource = datasource;
+  }
+
+  private TaskInfo()


This constructor could be removed if you implement the other suggestions in the review.

yes, will be removed.

gianm · 2018-06-11T16:03:56Z

+    return dataSource;
+  }
+
+  public static class TaskInfoBuilder


IMO, this builder isn't necessary, since TaskInfo instances seem to always be created with all four parameters provided up front. Builders are usually worth it when you have a lot of optional parameters, since they avoid the need to have constructors with huge numbers of nulls in them. But that is not the case here, so I'd suggest removing this builder.

As a side note, if this builder did continue to exist then I'd have a couple of comments about it:

The build() method should use the 4-arg TaskInfo constructor, which would allow TaskInfo to be an immutable class.

The build(String id, DateTime createdTime, TaskState state, String datasource) method shouldn't exist: it isn't really builder-like. It's more of a static factory method (the third major way to create objects; the three being constructors, builders, and static factory methods).

got rid of the builder

gianm · 2018-06-11T16:06:55Z

  List<StatusType> getInactiveStatusesSince(DateTime timestamp, @Nullable Integer maxNumStatuses);

+  /**
+   * Return up to {@code maxNumStatuses} {@link TaskInfo} objects for all inactive entries created on or later than the given timestamp


This line is a bit too long (we generally use 120 chars max).

splitted the comment

gianm · 2018-06-11T23:20:25Z

+    Function<TaskStatusPlus, Iterable<ResourceAction>> raGenerator = taskStatusPlus -> {
+      final String taskId = taskStatusPlus.getId();
+      //final Optional<Task> optionalTask = taskStorageQueryAdapter.getTask(taskId);
+      TaskInfo taskInfo = taskInfoCache.get(taskId);


Based on the comments above for taskInfoCache, you'll probably want to rethink how this works.

Maybe it would make sense to pass in a map of task id -> datasource (presumably freshly retrieved) to this securedTaskStatusPlus method?

gianm · 2018-06-11T23:36:11Z

+    }
+    TaskRunner runner = taskRunnerOpt.get();
+    Collection<? extends TaskRunnerWorkItem> runnersKnownTasks = runner.getKnownTasks();
+    List<String> runnerKnownTaskIds = runnersKnownTasks.stream()


Since you're calling contains on this a lot, it should be a Set. contains on Lists is generally O(n), whereas contains on Sets is generally O(log n) or O(1).

gianm · 2018-06-11T23:45:58Z

+
+    if (RunnerTaskState.PENDING.equals(state)) {
+      Collection<? extends TaskRunnerWorkItem> knownPendingTasks = runner.getPendingTasks();
+      List<String> pendingTaskIds = knownPendingTasks


This should also be a Set.

gianm · 2018-06-11T23:46:09Z

+
+    if (RunnerTaskState.RUNNING.equals(state)) {
+      Collection<? extends TaskRunnerWorkItem> knownRunningTasks = runner.getRunningTasks();
+      List<String> runningTaskIds = knownRunningTasks


This should also be a Set.

gianm · 2018-06-11T23:47:12Z

+      );
+    }
+    TaskRunner runner = taskRunnerOpt.get();
+    Collection<? extends TaskRunnerWorkItem> runnersKnownTasks = runner.getKnownTasks();


It looks like these variables are only needed when the state is RunnerTaskState.WAITING, and could be moved inside that block.

moved inside the block.

Fix a bug in securedTaskStatusPlus

jon-wei · 2018-06-14T20:53:21Z

+    );
+  }
+
+  class TaskInfoMapper implements ResultSetMapper


ResultSetMapper can take a type parameter, let's change this to:

class TaskInfoMapper implements ResultSetMapper<TaskInfo<EntryType>>

jon-wei · 2018-06-14T20:57:14Z

+    @Override
+    public TaskInfo<EntryType> map(int index, ResultSet resultSet, StatementContext context) throws SQLException
+    {
+      final TaskInfo taskInfo;


Can you add the EntryType type parameter to TaskInfo here?

jon-wei · 2018-06-14T20:57:56Z

+      final TaskInfo taskInfo;
+      try {
+        TaskStatus status = getJsonMapper().readValue(resultSet.getBytes("status_payload"), getStatusType());
+        taskInfo = new TaskInfo(


this can be taskInfo = new TaskInfo<>( after adding the EntryType parameter to TaskInfo above

jon-wei · 2018-06-14T21:59:22Z

+    return storage.getActiveTaskInfo();
+  }
+
+  public List<TaskStatus> getRecentlyFinishedTaskStatuses(


It looks like getRecentlyFinishedTaskStatuses could be removed, it's only called in tests and getRecentlyCompletedTaskInfo can provide a superset of that information

i thought i am missing some unused code, yes removed it and tests don't need it really, so removed from tests as well.

jon-wei · 2018-06-14T22:15:10Z

      );
    };
-
+    List<TaskStatusPlus> dataSourceFilter = collectionToFilter;


For the case where the caller provides a datasource, I think getTasks could be optimized in a couple of ways:

We could offload filtering of task by datasource to the metadata store, this would cut down on the amount of data being transferred to and processed on the overlord and also allow users to set up indices for filtering by datasource on the task table if needed

If we do that, we could additionally reduce the number of authorization checks needed from (number of tasks) to one (just check that that user has access to the datasource being filtered on once)

Do you mean passing in the datasource as argument to getActiveTaskInfo or getRecentlyCompletedTaskInfo and then filtering during the query ?

yes, we could add datasource as an argument to those methods to put in a "WHERE datasource = x" clause to the SQL queries being made

ok, added WHERE datasource = :ds clause to all sql implementations

jon-wei · 2018-06-14T22:22:43Z

@surekhasaharan Left some initial comments, still reviewing

Change TaskInfoMapper signature to add generic type

jon-wei · 2018-06-14T23:34:21Z

-      if (!optionalTask.isPresent()) {
+    Function<TaskStatusPlus, Iterable<ResourceAction>> raGenerator = taskStatusPlus -> {
+      final String taskId = taskStatusPlus.getId();
+      String taskDatasource = taskInfoCache.get(taskId);


I think we can remove the taskInfoCache, since it's purely local to getTasks now and it's only read from here (we can get the datasource from taskStatusPlus as well)

right, it can be removed.

jon-wei · 2018-06-14T23:35:39Z

        "    id, "
-        + "  status_payload "
+        + "  status_payload, "
+        + " created_date, "


nit: spacing on the new fields like created_date should align with status_payload

jon-wei · 2018-06-14T23:42:08Z

+  }
+
+  private List<AnyTask> filterActiveTasks(
+      HttpServletRequest req,


req is unused here, let's remove it

jon-wei · 2018-06-14T23:47:41Z

+      List<TaskInfo<Task>> allActiveTaskInfo
+  )
+  {
+    final List<AnyTask> allTasks = Lists.newArrayList();


Let's move allTasks and the for loop below to getTasks, so we only create this list once

good point, moved inside getTasks

* Passed datasource as argument to be used in sql query * Other minor fixes

…t-api

jon-wei

left some minor comments

jon-wei · 2018-06-15T20:24:05Z

-                maxTaskStatuses
-            )
+            DateTimes.nowUtc().minus(duration == null ? config.getRecentlyFinishedThreshold() : duration),
+            maxTaskStatuses, datasource


nit: let's put datasource on a separate line

jon-wei · 2018-06-15T20:25:17Z

-      for (TaskInfo taskInfo : taskInfoList) {
-        taskInfoCache.putIfAbsent(taskInfo.getId(), taskInfo.getDataSource());
-      }
+          maxCompletedTasks, duration, dataSource);


nit: can you move ); to a separate line

jon-wei · 2018-06-15T20:27:59Z

        + "  %s "
        + "WHERE "
-        + "  active = FALSE AND created_date >= :start "
+        + whereStmt


nit: whereStmt can be replaced with geWhereClause(datasource), we can get rid of the extra variable there

jon-wei · 2018-06-15T20:30:35Z

    return query;
  }
-
+  private String getWhereClause(@Nullable String datasource)


Suggest renaming getWhereClause to getWhereClauseForInactiveStatusesSinceQuery so it's clear what it's used for

*Some minor changes, rename method, spacing changes

…t-api

jon-wei · 2018-06-15T21:20:48Z

    };
-
+    List<TaskStatusPlus> dataSourceFilter = collectionToFilter;
+    if (dataSource != null) {


this block can be removed now since the datasource filtering is handled by the metadata store now

you can also have a more optimized authorization check path for the datasource != null case now, where you don't need to run the auth check on all the tasks returned (since they'll all have the same datasource), and just check that the user has access to the datasource once

makes sense, done.

…t-api

* Add max limit to getRecentlyFinishedTaskInfo in HeapMemoryTaskStorage * Add TaskLocation to Anytask object

jon-wei

LGTM

gianm · 2018-06-18T19:05:37Z

+
+    //check for valid state
+    if (state != null) {
+      List validStates = ImmutableList.of("pending", "waiting", "running", "complete");


It'd be more hygienic if this were a constant, like

public static final API_TASK_STATES = ImmutableList.of("pending", "waiting", "running", "complete")

sure, done.

gianm · 2018-06-18T19:06:22Z

+      List validStates = ImmutableList.of("pending", "waiting", "running", "complete");
+      if (!validStates.contains(StringUtils.toLowerCase(state))) {
+        return Response.status(Status.BAD_REQUEST)
+                       .entity("Bad state query param passed : " + state).entity(ImmutableList.of()).build();


I don't think you can do entity twice. I think the last one wins if you do this, so you should get rid of the second entity. And while doing that, you might as well make the message a little nicer, something like:

String.format("Invalid 'state', valid values are: " + API_TASK_STATES)

yeah, missed that there was entity twice here.

gianm · 2018-06-18T19:08:44Z

+          new Resource(dataSource, ResourceType.DATASOURCE),
+          Action.READ
+      );
+      Access authResult = AuthorizationUtils.authorizeResourceAction(


It's a little weird to mix final and non-final variables here. Either one is ok (we don't have a strong style guide for whether variables like this should be final or not) but you should at least be consistent within a method.

If it was me writing it then I would do final for both :)

yes final it is.

gianm · 2018-06-18T19:14:05Z

+    Optional<TaskRunner> taskRunnerOpt = taskMaster.getTaskRunner();
+    if (!taskRunnerOpt.isPresent()) {
+      throw new WebApplicationException(
+          Response.serverError().entity("No task runner found ").build()


Extraneous space here.

removed space

…t-api

gianm · 2018-06-19T01:08:40Z

@surekhasaharan from the test, might this be a real error?

Tests run: 19, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.268 sec <<< FAILURE! - in io.druid.indexing.overlord.http.OverlordResourceTest
testGetTasksNegativeState(io.druid.indexing.overlord.http.OverlordResourceTest)  Time elapsed: 0.007 sec  <<< ERROR!
java.lang.ClassCastException: java.lang.String cannot be cast to java.util.List
	at io.druid.indexing.overlord.http.OverlordResourceTest.testGetTasksNegativeState(OverlordResourceTest.java:750)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:168)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:367)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:274)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:161)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:290)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:242)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:121)

* Add the new tasks api in overlordResource It takes 4 optional query params * state(pending/running/waiting/compelte) * dataSource * interval (applies to completed tasks) * maxCompletedTasks (applies to completed tasks) If all params are null, the api returns all the tasks * Add the state to each task returned by tasks endpoint * divide active tasks into waiting, pending or running * Add more unit tests * Add UNKNOWN state to TaskState * Fix the authorization calls * WIP: PR comments Added new class to capture task info for caching Other refactoring * Refactoring : move TaskStatus class to druid-api so it can be accessed within server And other related classes like TaskState and TaskStatusPlus are in api * Remove unused class and apis accessing it * Add a separate cache for recently completed tasks This is to mainly capture the task type from payload * Ignore a test * Add a RuntimeTaskState to encompass all states a task can be in * Revert "Add a RuntimeTaskState to encompass all states a task can be in" This reverts commit 2a527a0. * Fix wrong api call * Fix and unignore tests * Remove waiting,pending state from TaskState * Add RunnerTaskState * Missed the annotation runnerStatusCode * Fix the creationTime * Fix the createdTime and queueInsertionTime for running/active tasks * Clean up tests * Add javadocs * Potentially fix the teamcity build * Address PR comments *Get rid of TaskInfoBuilder *Make TaskInfoMapper static nested class *Other changes * fix import in MaterializedViewSupervisor after merge * Address PR comments on * Replace global cache with local map * combine multiple queries into one * Removed unused code * Fix unit tests Fix a bug in securedTaskStatusPlus * Remove getRecentlyFinishedTaskStatuses method Change TaskInfoMapper signature to add generic type * Address PR comments * Passed datasource as argument to be used in sql query * Other minor fixes * Address PR comments *Some minor changes, rename method, spacing changes * Add early auth check if datasource is not null * Fix test case * Add max limit to getRecentlyFinishedTaskInfo in HeapMemoryTaskStorage * Add TaskLocation to Anytask object * Address PR comments * Fix a bug in test case causing ClassCastException

Surekha Saharan added 4 commits May 22, 2018 13:37

Add the new tasks api in overlordResource

5a51b59

It takes 4 optional query params * state(pending/running/waiting/compelte) * dataSource * interval (applies to completed tasks) * maxCompletedTasks (applies to completed tasks) If all params are null, the api returns all the tasks

Add the state to each task returned by tasks endpoint

c62ee5d

* divide active tasks into waiting, pending or running * Add more unit tests

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

f9c9720

…t-api

Add UNKNOWN state to TaskState

2fddf7a

gianm added the Feature label May 24, 2018

Fix the authorization calls

d0019fb

jihoonson reviewed May 25, 2018

View reviewed changes

gianm reviewed May 25, 2018

View reviewed changes

surekhasaharan changed the title ~~Tasks endpoint api~~ WIP: Tasks endpoint api May 31, 2018

Surekha Saharan added 14 commits May 30, 2018 20:24

WIP: PR comments

129ecac

Added new class to capture task info for caching Other refactoring

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

220e90b

…t-api

Refactoring : move TaskStatus class to druid-api

7f79fa3

so it can be accessed within server And other related classes like TaskState and TaskStatusPlus are in api

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

e131f60

…t-api

Remove unused class and apis accessing it

2bd7e36

Add a separate cache for recently completed tasks

087148e

This is to mainly capture the task type from payload

Ignore a test

f7b1408

Add a RuntimeTaskState to encompass all states a task can be in

2a527a0

Revert "Add a RuntimeTaskState to encompass all states a task can be in"

0d84d87

This reverts commit 2a527a0.

Fix wrong api call

e4ad296

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

ab2d9e9

…t-api

Fix and unignore tests

dc1ffa6

Remove waiting,pending state from TaskState

9e668dc

* Add RunnerTaskState

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

ac86c55

…t-api

surekhasaharan changed the title ~~WIP: Tasks endpoint api~~ Tasks endpoint api Jun 5, 2018

Surekha Saharan added 5 commits June 5, 2018 16:59

Missed the annotation runnerStatusCode

64622e0

Fix the creationTime

b819df2

* Fix the createdTime and queueInsertionTime for running/active tasks * Clean up tests

Add javadocs

715538b

Potentially fix the teamcity build

c1190c8

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

ff34b67

…t-api

gianm reviewed Jun 11, 2018

View reviewed changes

Fix unit tests

e83a357

Fix a bug in securedTaskStatusPlus

jon-wei reviewed Jun 14, 2018

View reviewed changes

Remove getRecentlyFinishedTaskStatuses method

ed38889

Change TaskInfoMapper signature to add generic type

jon-wei reviewed Jun 14, 2018

View reviewed changes

Surekha Saharan added 2 commits June 15, 2018 11:27

Address PR comments

a5c6f3a

* Passed datasource as argument to be used in sql query * Other minor fixes

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

ae0dbfb

…t-api

jon-wei reviewed Jun 15, 2018

View reviewed changes

Surekha Saharan added 2 commits June 15, 2018 13:50

Address PR comments

60b2818

*Some minor changes, rename method, spacing changes

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

26150b1

…t-api

jon-wei reviewed Jun 15, 2018

View reviewed changes

Surekha Saharan added 3 commits June 15, 2018 15:41

Add early auth check if datasource is not null

5bd90b0

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

bc334ed

…t-api

Fix test case

d6198ea

* Add max limit to getRecentlyFinishedTaskInfo in HeapMemoryTaskStorage * Add TaskLocation to Anytask object

surekhasaharan changed the title ~~Tasks endpoint api~~ Improve task retrieval APIs on Overlord Jun 16, 2018

jon-wei approved these changes Jun 18, 2018

View reviewed changes

gianm reviewed Jun 18, 2018

View reviewed changes

Surekha Saharan added 2 commits June 18, 2018 13:03

Address PR comments

7ac1275

Merge branch 'master' of github.com:druid-io/druid into tasks-endpoin…

6204974

…t-api

gianm approved these changes Jun 18, 2018

View reviewed changes

Fix a bug in test case causing ClassCastException

085dce0

gianm merged commit 8619adb into apache:master Jun 19, 2018

surekhasaharan deleted the tasks-endpoint-api branch June 19, 2018 20:32

dclim added this to the 0.13.0 milestone Oct 8, 2018

dclim mentioned this pull request Oct 10, 2018

Druid 0.13.0-incubating release notes #6442

Closed

FrankChen021 mentioned this pull request Aug 14, 2021

Cannot retrieve proper tasks list from Overlord Task API with createdTimeInterval paramater #11589

Open

Conversation

surekhasaharan commented May 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

surekhasaharan Jun 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

surekhasaharan commented May 24, 2018 •

edited

Loading

surekhasaharan Jun 1, 2018 •

edited

Loading