Add start time and duration to tasks #16829
Conversation
imotov
added
>enhancement
review
v5.0.0-alpha1
:Distributed/Task Management
| @@ -50,17 +51,24 @@ | |||
|
|
|||
| private final String description; | |||
|
|
|||
| private final long startTime; | |||
|
|
|||
| private final long runningTime; | |||
There was a problem hiding this comment.
I'm not sure we need to send this over the wire though.
There was a problem hiding this comment.
We either have to send it over the wire or synchronize clocks on all nodes. Otherwise, users might get negative running times, which just look weird. I think sending this over the wire is a simpler solution.
There was a problem hiding this comment.
++ Leave a comment to that effect?
|
Hurray! I left some minor comments, all of which you can feel free to say "no, I like it the way it is, thank you". I think it'd tell a great story if you added "get me tasks that have been running longer than X seconds" to the task list API in this PR. It'd make it super obvious why this is important. But this PR is fine as is if you want. |
|
To make it official, LGTM. |
|
@nik9000 I pushed changes to address your comments. What do you think? |
| @@ -50,17 +51,24 @@ | |||
|
|
|||
| private final String description; | |||
|
|
|||
| private final long startTime; | |||
There was a problem hiding this comment.
Maybe these should be TimeValues?
There was a problem hiding this comment.
Err, the next one a TimeValue, this one a DateTime or something? Its not required, at all and I just thought of it but a TimeValue would make runningTimeNanos easier to read I think.
There was a problem hiding this comment.
This is just a transport format, which is easier to work with. We do convert this long to a TimeValue in XContentBuilder#timeValueField() if a user requests human readable output. It also happens to dates in XContentBuilder#dateValueField(). So, I don't think storing it here in milliseconds and nanoseconds will affect readability.
|
Fair enough. LGTM. |
Tasks now contain timestamps indicating when the tasks were created and current run time
imotov
force-pushed
the
change-tasks-rest-api
branch
from
c55926d
to
863fab4
Compare
Tasks now contain timestamps indicating when the tasks were created and current run time