Cancel reindex returns same response as GET

[szybia]

• Add more fully-featured response on POST _reindex/{task_id}/_cancel?wait_for_completion=true, similar to what is returned in GET _reindex/{task_id} since it's handy to have without doing a follow-up GET
• ?wait_for_completion=false returns the existing acknowledge: true response

Closes elastic/elasticsearch-team#2086

[szybia]

incoming yaml rest tests (next PR) will assert response more definitively

for now, just asserting the basics that are relevant for these tests

[szybia]

there's a choice to be made here that we discussed on the meeting, detailing so we can make a concrete decision.

---

firstly, context/existing art:

• cancelled field is not recorded in tasks index, so a GET after cancellation seems to default to default value (false)
• POST _tasks/:taskId/_cancel?wait_for_completion=true&filter_path=nodes.*.tasks
  • Note the cancelled: true and absence of response/full-featured response

    {
      "nodes": {
        "1RCnK_OnSamzfgmhcshBrA": {
          "tasks": {
            "1RCnK_OnSamzfgmhcshBrA:7281": {
              "node": "1RCnK_OnSamzfgmhcshBrA",
              "id": 7281,
              "type": "transport",
              "action": "indices:data/write/reindex",
              "start_time_in_millis": 1768412466161,
              "running_time_in_nanos": 6862322167,
              "cancellable": true,
              "cancelled": true,
              "headers": {}
            }
          }
        }
      }
    }
    

• GET _tasks/:taskId
  • Returns fully-featured response (task, slices, response, response.failures) and notably cancelled: false

---

in terms of what we're doing here, I see two main choices to get what we want (a more fully-featured response on POST _reindex/{task_id}/_cancel?wait_for_completion=true)

• what i currently have here: we generate the taskInfo here
  • pros:
    • cancelled: true (makes sense)
    • slightly simpler code
    • still get status.slices information, same as GET _reindex/{task_id}
  • cons
    • running_time_in_nanos is calculated right here System.nanoTime() - start, so this will be slightly different from what we will see in GET _reindex/{task_id} (taskInfo generated elsewhere)
    • don't get response, response.failures objects
• do a GET _reindex/{task_id} transport call
  • pros:
    • get exact same response as GET _reindex/{task_id}, including response
    • building on top of GET
  • cons:
    • cancelled: false, confusing

other potential avenues:

• we could get the full response using GET, and hackily flip cancelled to true
• i could investigate further whether any way to get from cancelTaskAndDescendants the full TaskResult that will be stored in tasks index, but assuming async stuff will make it hard to hand off

---

stating preference:

• i'm leaning towards saying what i have here is good enough, on first glance response field returned in GET doesn't seem to have extra useful info other than what is here already within slices, maybe response.failures from the BulkByScroll
• in the unlikely case this is wanted, you can just do a folllow-up GET
• in previous conversations we didn't seem too bothered about having response but we included it (from what i read, because why not)

---

attaching below two files so reviewers can diff what is returned in POST _reindex/{task_id}/_cancel?wait_for_completion=true, and GET _reindex/{task_id} after cancellation

• get.json
• cancel.json

[PeteGillinElastic]

I think I'm happy with what you have here. As discussed, I'm okay with not including the full response. I think I'm also okay with the times not matching up perfectly, especially as they're in different units. Reporting the correct value for cancelled is probably a good thing. The only thing that's giving me pause is that we're not consistent with get tasks... but there's no real reason we should be, since we're trying to mostly obfuscate the fact that this is wrappers around tasks (and esp. since that is doing something confusing around the cancelled field.)

[elasticsearchmachine]

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

[PeteGillinElastic]

I think this LGTM, although I'd also be interested to see if @samxbr agrees.

[PeteGillinElastic]

I think I'm happy with what you have here. As discussed, I'm okay with not including the full response. I think I'm also okay with the times not matching up perfectly, especially as they're in different units. Reporting the correct value for cancelled is probably a good thing. The only thing that's giving me pause is that we're not consistent with get tasks... but there's no real reason we should be, since we're trying to mostly obfuscate the fact that this is wrappers around tasks (and esp. since that is doing something confusing around the cancelled field.)

[PeteGillinElastic]

I think I'm okay with this (see below) but I'd also be interested to know what @samxbr thinks.

[samxbr]

LGTM!