zachjsh opened a new pull request, #16127: URL: https://github.com/apache/druid/pull/16127
Fixes https://github.com/apache/druid/issues/16125 ### Description This fixes an issue in which an unqueryable datasource, cannot have its segments mark used. Such a datasource may become unqueryable because all of segments were previously marked as unused. This bug effectively did not allow a user to recover a datasource that had all segments soft-deleted (mark unused). The issue was that there was a check that short circuits the work to mark segments used if it thinks that the datasource does not exist. However, if all of the data in the table has been soft deleted (markUnused), then Druid thinks the datasource doesnt exist, which consequently means that if all data in a datasource in soft deleted, it cannot be recovered. It seems that this check has been here since the inception of the markUsed api https://github.com/apache/druid/pull/7490. My thought is that the check for datasource existence was copied from the MarkAsUnused api logic, and used to save potentially unnecessary metadata ops . For markingAsUnused, I believe it does make sense to check for queryable datasource, because if there are any segments to mark as unused, then they are presumably used now, which would make the datasource queryable. However this same check is not appropriate when marking segments used, as there may be no queryable segments. #### Release note <!-- Give your best effort to summarize your changes in a couple of sentences aimed toward Druid users. If your change doesn't have end user impact, you can skip this section. For tips about how to write a good release note, see [Release notes](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#release-notes). --> <hr> ##### Key changed/added classes in this PR * `MyFoo` * `OurBar` * `TheirBaz` <hr> <!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. --> This PR has: - [ ] 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) - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [ ] 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. - [ ] 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]
