georgew5656 opened a new pull request, #16065:
URL: https://github.com/apache/druid/pull/16065
Fixes a reporting bug with running k8s compaction tasks on azure.
### Description
There is logic in AbstractTask.cleanup that lets peons that are running
standalone (currently only a thing with k8s mode) push report.json and a
status.json after they have completed running. The report.json is a regular
task report and the status.json is used by the KubernetesTaskRunner to check
the status of the task.
This logic assumes that each task is run in isolation, e.g. that a peon will
only run a single task. This assumption is not true in two cases, one case
where a IndexTask is initialized and run inside a ParallelIndexSupervisorTask
and one case where onr or many ParallelIndexSupervisorTask's are initialized
and run inside a CompactionTask.
The problem with these cases is that both the interior and wrapper tasks try
to run cleanup and push their report.json/status.json.
Currently this works okay if using amazon s3 as the backend, becasue the s3
client by default overwrites files and the wrapper task pushes its reports
last, so the status of task will always be whatever the wrapper task says.
The AzureStorage client we wrote doesn't overwrite, so the wrapper task will
actually throw a exception when trying to write its reports. The task ends up
succeeding anyways because the KubernetesTaskRunner will see the k8s job exit
and check status.json (which is successful since the interior task succeeded
and pushed its status.json), but this is undesirable behavior.
#### Fixed the bug ...
#### Renamed the class ...
#### Added a forbidden-apis entry ...
The IndexTask already has a flag for telling it whether it is running
standalone or inside another task and check this flag when deciding whether to
cleanup. I pulled some of this logic into AbstractTask (since that is where the
pushing reports logic is anyways), and added a interface on all tasks called
shouldCleanup which defaults to true. The indexTask will return
shouldCleanup=false if it is running inside another task. Similarily,
ParallelIndexSupervisorTask will also return shouldCleanup=false if it is
running inside a compaction task.
I also updated the azure client to overwrite files by default because this
seems to be the default behavior in object stores and it's easier to keep
things consistent.
#### Release note
Fixing bug with kubernetes ingestion and compaction
##### Key changed/added classes in this PR
* `AbstractTask`
* `IndexTask`
* `ParallelIndexSuperivsorTask`
* `AzureStorage`
This PR has:
- [X] been self-reviewed.
- [ ] using the [concurrency
checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md)
(Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in
[licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [X] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [X] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [ ] added integration tests.
- [X] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
