Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-6649: Should catch OutOfRangeException for ReplicaFetcherThread #4707

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

huxihx
Copy link
Contributor

@huxihx huxihx commented Mar 14, 2018

https://issues.apache.org/jira/browse/KAFKA-6649

AbstractFetcherThread.processFetchRequest should catch OffsetOutOfRangeException lest the thread was forcibly stopped.

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

https://issues.apache.org/jira/browse/KAFKA-6649

`AbstractFetcherThread.processFetchRequest` should catch OffsetOutOfRangeException lest the thread was forcibly stopped.
@huxihx
Copy link
Contributor Author

huxihx commented Mar 14, 2018

@rajinisivaram Could you please help review this minor change?

@rajinisivaram
Copy link
Contributor

@huxihx Thanks for the PR. I am not very familiar with that class, perhaps @junrao could review this.

@hachikuji
Copy link
Contributor

@huxihx One question to consider is whether this situation is still possible after KAFKA-3978 has been fixed.

@huxihx
Copy link
Contributor Author

huxihx commented Mar 15, 2018

I am not sure if KAFKA-3978 strictly closed the door to throw this exception. However, this patch works as a defensive strategy to make sure fetch thread will never unexpectedly be shut down due to this exception. Does it make any sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants