Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding task cancellation timestamp in task API #7445

Merged
merged 16 commits into from
May 23, 2023

Conversation

sgup432
Copy link
Contributor

@sgup432 sgup432 commented May 5, 2023

Description

Adding task cancellation start time and running time since cancellation details as discussed here - #6953

Related Issues

Partially resolves - #6953

Manual testing

Spun up an opensearch cluster and added deliberate delay in search request via sleep to mock and test this functionality.
Without cancellation

curl -XGET localhost:9200/_tasks?pretty
{
  "nodes" : {
    "2t7D3gtQQoydmYbreSltTw" : {
      "name" : "runTask-0",
      "transport_address" : "127.0.0.1:9300",
      "host" : "127.0.0.1",
      "ip" : "127.0.0.1:9300",
      "roles" : [
        "cluster_manager",
        "data",
        "ingest",
        "remote_cluster_client"
      ],
      "attributes" : {
        "testattr" : "test",
        "shard_indexing_pressure_enabled" : "true"
      },
      "tasks" : {
        "2t7D3gtQQoydmYbreSltTw:90" : {
          "node" : "2t7D3gtQQoydmYbreSltTw",
          "id" : 90,
          "type" : "transport",
          "action" : "cluster:monitor/tasks/lists",
          "start_time_in_millis" : 1683265864323,
          "running_time_in_nanos" : 248904,
          "cancellable" : false,
          "cancelled" : false,
          "headers" : { }
        },
        "2t7D3gtQQoydmYbreSltTw:91" : {
          "node" : "2t7D3gtQQoydmYbreSltTw",
          "id" : 91,
          "type" : "direct",
          "action" : "cluster:monitor/tasks/lists[n]",
          "start_time_in_millis" : 1683265864323,
          "running_time_in_nanos" : 116795,
          "cancellable" : false,
          "cancelled" : false,
          "parent_task_id" : "2t7D3gtQQoydmYbreSltTw:90",
          "headers" : { }
        },
        "2t7D3gtQQoydmYbreSltTw:67" : {
          "node" : "2t7D3gtQQoydmYbreSltTw",
          "id" : 67,
          "type" : "direct",
          "action" : "indices:data/read/search[phase/fetch/id]",
          "start_time_in_millis" : 1683265829337,
          "running_time_in_nanos" : 34986856773,
          "cancellable" : true,
          "cancelled" : false,
          "parent_task_id" : "2t7D3gtQQoydmYbreSltTw:61",
          "headers" : { }
        },
        "2t7D3gtQQoydmYbreSltTw:61" : {
          "node" : "2t7D3gtQQoydmYbreSltTw",
          "id" : 61,
          "type" : "transport",
          "action" : "indices:data/read/search",
          "start_time_in_millis" : 1683265829284,
          "running_time_in_nanos" : 35039394343,
          "cancellable" : true,
          "cancelled" : false,
          "headers" : { }
        }
      }
    }
  }
}

After cancellation parent Search Task

curl -XGET localhost:9200/_tasks?pretty
{
  "nodes" : {
    "2t7D3gtQQoydmYbreSltTw" : {
      "name" : "runTask-0",
      "transport_address" : "127.0.0.1:9300",
      "host" : "127.0.0.1",
      "ip" : "127.0.0.1:9300",
      "roles" : [
        "cluster_manager",
        "data",
        "ingest",
        "remote_cluster_client"
      ],
      "attributes" : {
        "testattr" : "test",
        "shard_indexing_pressure_enabled" : "true"
      },
      "tasks" : {
        "2t7D3gtQQoydmYbreSltTw:164" : {
          "node" : "2t7D3gtQQoydmYbreSltTw",
          "id" : 164,
          "type" : "direct",
          "action" : "indices:data/read/search[phase/fetch/id]",
          "start_time_in_millis" : 1683265979351,
          "running_time_in_nanos" : 24063541772,
          "cancellable" : true,
          "cancelled" : true,
          "parent_task_id" : "2t7D3gtQQoydmYbreSltTw:61",
          "headers" : { },
          "cancelled_at_millis" : 1683265995821
        },
        "2t7D3gtQQoydmYbreSltTw:181" : {
          "node" : "2t7D3gtQQoydmYbreSltTw",
          "id" : 181,
          "type" : "transport",
          "action" : "cluster:monitor/tasks/lists",
          "start_time_in_millis" : 1683266003414,
          "running_time_in_nanos" : 243712,
          "cancellable" : false,
          "cancelled" : false,
          "headers" : { }
        },
        "2t7D3gtQQoydmYbreSltTw:61" : {
          "node" : "2t7D3gtQQoydmYbreSltTw",
          "id" : 61,
          "type" : "transport",
          "action" : "indices:data/read/search",
          "start_time_in_millis" : 1683265829284,
          "running_time_in_nanos" : 174130192988,
          "cancellable" : true,
          "cancelled" : true,
          "headers" : { },
          "cancelled_at_millis" : 1683265995820
        },
        "2t7D3gtQQoydmYbreSltTw:182" : {
          "node" : "2t7D3gtQQoydmYbreSltTw",
          "id" : 182,
          "type" : "direct",
          "action" : "cluster:monitor/tasks/lists[n]",
          "start_time_in_millis" : 1683266003415,
          "running_time_in_nanos" : 120782,
          "cancellable" : false,
          "cancelled" : false,
          "parent_task_id" : "2t7D3gtQQoydmYbreSltTw:181",
          "headers" : { }
        }
      }
    }
  }
}

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@sgup432 sgup432 changed the title Adding task cancellation time in task API Adding task cancellation timestamp in task API May 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testReplicaHasDiffFilesThanPrimary

@codecov-commenter
Copy link

codecov-commenter commented May 8, 2023

Codecov Report

Merging #7445 (2d43f9e) into main (b4b3724) will increase coverage by 70.72%.
The diff coverage is 80.00%.

@@             Coverage Diff             @@
##             main    #7445       +/-   ##
===========================================
+ Coverage        0   70.72%   +70.72%     
- Complexity      0    56132    +56132     
===========================================
  Files           0     4680     +4680     
  Lines           0   266104   +266104     
  Branches        0    39070    +39070     
===========================================
+ Hits            0   188192   +188192     
- Misses          0    61938    +61938     
- Partials        0    15974    +15974     
Impacted Files Coverage Δ
...r/src/main/java/org/opensearch/tasks/TaskInfo.java 89.37% <73.33%> (ø)
...ain/java/org/opensearch/tasks/CancellableTask.java 82.60% <83.33%> (ø)
...erver/src/main/java/org/opensearch/tasks/Task.java 80.90% <100.00%> (ø)

... and 4677 files with indirect coverage changes

CHANGELOG.md Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchTaskCancellationWithHighCpu

Copy link
Collaborator

@Bukhtawar Bukhtawar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor comment. Otherwise LGTM

Copy link
Collaborator

@Bukhtawar Bukhtawar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor comment. Otherwise LGTM

Copy link
Collaborator

@Bukhtawar Bukhtawar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor comment. Otherwise LGTM

@andrross
Copy link
Member

@dblock Can you take a look? I think the thing you requested changes on has been addressed.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's walk back from suggestions and help this PR get in. What are your must haves @Bukhtawar @andrross?

@andrross
Copy link
Member

andrross commented May 23, 2023

@dblock Personally I'm fine with it as-is. Just waiting on resolving your request changes :) I do think the @Nullable annotation is a good idea now that you called it out. I just pushed a merge commit from main that should fix the gradle check errors.

@Bukhtawar
Copy link
Collaborator

No strong opinion from my side as well. There is only a small overhead of the non-primitive Long wrt to the primitive long. That should be fine as long as it aids readability

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchShardTaskCancellationWithHighCpu

@dblock dblock merged commit 8d7a544 into opensearch-project:main May 23, 2023
@dblock dblock added the backport 2.x Backport to 2.x branch label May 23, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 23, 2023
* Adding task cancellation time in task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing unit tests and addressing comments

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Adding change log for unreleased 2.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing running time cancel info from task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Replacing long primitive with Long object

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Making cancelledAt field human readable

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing failing test

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing the feature from unreleased 3.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing ListTasksResponseTests failure

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Test failure fix

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Changing naming convention to cancellationStartTime

Signed-off-by: Sagar Upadhyaya <[email protected]>

---------

Signed-off-by: Sagar Upadhyaya <[email protected]>
Signed-off-by: Sagar <[email protected]>
Co-authored-by: Andrew Ross <[email protected]>
(cherry picked from commit 8d7a544)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
andrross added a commit that referenced this pull request May 24, 2023
* Adding task cancellation timestamp in task API (#7445)

* Adding task cancellation time in task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing unit tests and addressing comments

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Adding change log for unreleased 2.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing running time cancel info from task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Replacing long primitive with Long object

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Making cancelledAt field human readable

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing failing test

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing the feature from unreleased 3.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing ListTasksResponseTests failure

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Test failure fix

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Changing naming convention to cancellationStartTime

Signed-off-by: Sagar Upadhyaya <[email protected]>

---------

Signed-off-by: Sagar Upadhyaya <[email protected]>
Signed-off-by: Sagar <[email protected]>
Co-authored-by: Andrew Ross <[email protected]>
(cherry picked from commit 8d7a544)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Changing version guard to 2.8

Signed-off-by: Sagar Upadhyaya <[email protected]>

---------

Signed-off-by: Sagar Upadhyaya <[email protected]>
Signed-off-by: Sagar <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andrew Ross <[email protected]>
dblock pushed a commit to dblock/OpenSearch that referenced this pull request May 25, 2023
* Adding task cancellation time in task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing unit tests and addressing comments

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Adding change log for unreleased 2.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing running time cancel info from task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Replacing long primitive with Long object

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Making cancelledAt field human readable

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing failing test

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing the feature from unreleased 3.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing ListTasksResponseTests failure

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Test failure fix

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Changing naming convention to cancellationStartTime

Signed-off-by: Sagar Upadhyaya <[email protected]>

---------

Signed-off-by: Sagar Upadhyaya <[email protected]>
Signed-off-by: Sagar <[email protected]>
Co-authored-by: Andrew Ross <[email protected]>
suranjay pushed a commit to suranjay/OpenSearch that referenced this pull request May 29, 2023
* Adding task cancellation time in task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing unit tests and addressing comments

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Adding change log for unreleased 2.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing running time cancel info from task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Replacing long primitive with Long object

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Making cancelledAt field human readable

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing failing test

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing the feature from unreleased 3.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing ListTasksResponseTests failure

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Test failure fix

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Changing naming convention to cancellationStartTime

Signed-off-by: Sagar Upadhyaya <[email protected]>

---------

Signed-off-by: Sagar Upadhyaya <[email protected]>
Signed-off-by: Sagar <[email protected]>
Co-authored-by: Andrew Ross <[email protected]>
stephen-crawford pushed a commit to stephen-crawford/OpenSearch that referenced this pull request May 31, 2023
* Adding task cancellation time in task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing unit tests and addressing comments

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Adding change log for unreleased 2.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing running time cancel info from task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Replacing long primitive with Long object

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Making cancelledAt field human readable

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing failing test

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing the feature from unreleased 3.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing ListTasksResponseTests failure

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Test failure fix

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Changing naming convention to cancellationStartTime

Signed-off-by: Sagar Upadhyaya <[email protected]>

---------

Signed-off-by: Sagar Upadhyaya <[email protected]>
Signed-off-by: Sagar <[email protected]>
Co-authored-by: Andrew Ross <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
* Adding task cancellation time in task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing unit tests and addressing comments

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Adding change log for unreleased 2.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing running time cancel info from task API

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Replacing long primitive with Long object

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Making cancelledAt field human readable

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing failing test

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Removing the feature from unreleased 3.x

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Fixing ListTasksResponseTests failure

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Test failure fix

Signed-off-by: Sagar Upadhyaya <[email protected]>

* Changing naming convention to cancellationStartTime

Signed-off-by: Sagar Upadhyaya <[email protected]>

---------

Signed-off-by: Sagar Upadhyaya <[email protected]>
Signed-off-by: Sagar <[email protected]>
Co-authored-by: Andrew Ross <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants