Skip to content

Fix NPE for taskGroupId when rolling update#6168

Merged
fjy merged 6 commits intoapache:masterfrom
jihoonson:fix-npe-taskGroupId
Aug 17, 2018
Merged

Fix NPE for taskGroupId when rolling update#6168
fjy merged 6 commits intoapache:masterfrom
jihoonson:fix-npe-taskGroupId

Conversation

@jihoonson
Copy link
Copy Markdown
Contributor

@jihoonson jihoonson commented Aug 13, 2018

Fixes #6163.

@jihoonson jihoonson added this to the 0.12.3 milestone Aug 13, 2018
nishantmonu51
nishantmonu51 reviewed Aug 13, 2018
View reviewed changes
Comment thread indexing-service/src/main/java/io/druid/indexing/overlord/supervisor/SupervisorManager.java Outdated
String supervisorId,
int taskGroupId,
@Nullable String taskId,
Integer taskGroupId,
Copy link
Copy Markdown
Member

@nishantmonu51 nishantmonu51 Aug 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should taskGroupId be nullable instead of taskId ?

Copy link
Copy Markdown
Contributor Author

@jihoonson jihoonson Aug 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Fixed.

@jihoonson jihoonson mentioned this pull request Aug 13, 2018
Closed
@pdeva
Copy link
Copy Markdown
Contributor

pdeva commented Aug 13, 2018

note that this issue can occur well after the rolling update. we are seeing this on our fully updated cluster

@jihoonson
Copy link
Copy Markdown
Contributor Author

jihoonson commented Aug 13, 2018

@pdeva this error should never happen once the cluster is fully updated. Do you have the overlord and task logs? If you have, please add them to #6163.

@pdeva
Copy link
Copy Markdown
Contributor

pdeva commented Aug 13, 2018 •
edited
Loading

replied on druid-dev thread. they have recovered for now but will indeed post logs if it occurs again.

@jihoonson
Copy link
Copy Markdown
Contributor Author

jihoonson commented Aug 13, 2018

@pdeva thanks.

BTW, this patch is incomplete. I think we need a way to find taskGroup using baseSequenceName to fully support rolling update. I'll update this PR soon.

@jihoonson jihoonson added the WIP label Aug 13, 2018
@jihoonson jihoonson removed the WIP label Aug 14, 2018
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Aug 17, 2018

👍

@fjy fjy merged commit 2bfe1b6 into apache:master Aug 17, 2018
jon-wei pushed a commit to jon-wei/druid that referenced this pull request Aug 18, 2018
@jihoonson @jon-wei
[Backport] Fix NPE for taskGroupId when rolling update (apache#6168)
4e44af1
@jon-wei jon-wei mentioned this pull request Aug 18, 2018
Merged
jon-wei added a commit that referenced this pull request Aug 18, 2018
@jon-wei
[Backport] Fix NPE for taskGroupId when rolling update (#6168) (#6188)
bf1d5d7
jon-wei added a commit to implydata/druid-public that referenced this pull request Aug 18, 2018
@jon-wei
[Backport] Fix NPE for taskGroupId when rolling update (apache#6168) (a…
23731f0 
…pache#6188)
@jon-wei jon-wei mentioned this pull request Sep 1, 2018
Closed
@jihoonson jihoonson mentioned this pull request Oct 21, 2019
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nishantmonu51 nishantmonu51 nishantmonu51 left review comments

Assignees

No one assigned

Labels

Area - Streaming Ingestion Bug

Projects

None yet

Milestone

0.12.3

Development

Successfully merging this pull request may close these issues.

4 participants

@jihoonson @pdeva @fjy @nishantmonu51