georgew5656 opened a new pull request, #17509:
URL: https://github.com/apache/druid/pull/17509
Fixes a bug around segment publishing when multiple tasks in the same task
group are pending completion.
### Description
It's possible for the following timeline to occur for a single supervisor.
Task 1 goes into publishing, Task 2 is started
Supervisor is updated or handoff is triggerred manually
Task 2 goes into publishing and immediately publishes (it has not read many
events).
Task 2 fails because the offsets in the DB do not match (Task 1 has not
completed its SegmentTransactionalInsertAction yet).
Normally task 2 will fail with one of the below errors:
`The new start metadata state[%s] is ahead of the last committed"
+ " end state[%s]. Try resetting the supervisor`
`Inconsistency between stored metadata state[%s] and target state[%s]. Try
resetting the supervisor.`
Eventually Task 1 will publish and the supervisor will get itself sorted out
but this can lead to several minutes of downtime and lag (and many failed
tasks) as the supervisor tries to recover.
This fix attempts to address this issue by having the
SegmentTransactionalInsertAction check whether a task can actually publish
before trying.
#### Fixed the bug ...
#### Renamed the class ...
#### Added a forbidden-apis entry ...
The general approach here is to introduce a canPublishSegments method on
each supervisor, and then call this method each time a streaming task wants to
publish. If this method returns false, then throw a 503 error that the task
will then retry.
A alternative approach might be to only try this as a fallback (right now
this logic has to be run on every publish). But I thought this might introduce
consistency bugs since part of the publish logic would have ran and failed and
the logic introduced here is relatively simple (checking stuff in memory). I
can update the logic to be exception handling only if others have strong
opinions.
The canPublishSegments logic is only implemented on SeekableStreamSupervisor
and essentially looks at pendingCompletionTaskGroups to see if there is already
a task publishing (if the task is in pendingCompletionTaskGroups, that
indicates it is already publishing) with earlier offsets then the task that is
trying to publish.
I'm still working on some unit tests for this code.
#### Release note
Fix a bug that can occur when resubmitting supervisors or when task
publishing takes a long time.
##### Key changed/added classes in this PR
* `SeekableStreamSupervisor`
* `SupervisorManager`
* `SegmentTransactionalInsertAction`
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.
- [ ] 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]
