Fix .tasks index strict mapping: parent_id should be parent_task_id#48393
Fix .tasks index strict mapping: parent_id should be parent_task_id#48393javanna merged 3 commits intoelastic:masterfrom
Conversation
The .tasks index has mappings that's strictly defined. `parent_task_id` was defined as `parent_id` though which would cause an exception in case a task is persisted that has a parent task id set. While at it, a couple of compiler warnings were addressed and a test request builder was removed in favour of using its corresponding request.
|
Pinging @elastic/es-distributed (:Distributed/Task Management) |
|
run elasticsearch-ci/1 |
|
run elasticsearch-ci/default-distro |
|
run elasticsearch-ci/bwc |
henningandersen
left a comment
There was a problem hiding this comment.
Thanks for looking into this @javanna. I left a single drive-by comment.
| "type": "long" | ||
| }, | ||
| "parent_id": { | ||
| "parent_task_id": { |
There was a problem hiding this comment.
I think we need to increment the version in TaskResultsService.TASK_RESULT_MAPPING_VERSION in order to get the new mapping applied.
There was a problem hiding this comment.
good point, I was not aware of that. Do you know if there are tests around applying the updated mappings?
There was a problem hiding this comment.
I think not, since there would at least be no verification that the parent_task_id field is in the index after/during a rolling upgrade. I also seem to remember that some of the test frameworks delete the .tasks index between each test (but not sure if it applies to all upgrade tests).
jimczi
left a comment
There was a problem hiding this comment.
I don't see a case where a parent task would be set on a task that is persisted but this would save some time if this is needed in the future so LGTM.
|
@elasticmachine update branch |
|
run elasticsearch-ci/packaging-sample-matrix |
…48393) * Fix .tasks index strict mapping: parent_id should be parent_task_id The .tasks index has mappings that's strictly defined. `parent_task_id` was defined as `parent_id` though which would cause an exception in case a task is persisted that has a parent task id set. While at it, a couple of compiler warnings were addressed and a test request builder was removed in favour of using its corresponding request. * increment version
The built-in task index mapping has a version field in its metadata, so that the TaskResultsService can check to see if it needs to update mappings when a new task result is stored. #48393 updated this version in TaskResultsService but omitted to change the version in the mapping itself, so a mapping update is applied every time a new task result is stored. This commit updates the mapping version so that it corresponds to the version in TaskResultsService.
The built-in task index mapping has a version field in its metadata, so that the TaskResultsService can check to see if it needs to update mappings when a new task result is stored. #48393 updated this version in TaskResultsService but omitted to change the version in the mapping itself, so a mapping update is applied every time a new task result is stored. This commit updates the mapping version so that it corresponds to the version in TaskResultsService.
The .tasks index has mappings that's strictly defined.
parent_task_idwas defined as
parent_idthough which would cause an exception in casea task is persisted that has a parent task id set.
While at it, a couple of compiler warnings were addressed and a test
request builder was removed in favour of using its corresponding request.