Skip to content

Fixes bug in kill task scheduling DruidCoordinatorSegmentKiller.findIntervalForKillTask()#2346

Merged
nishantmonu51 merged 2 commits intoapache:masterfrom
himanshug:fix_bug
Jan 28, 2016
Merged

Fixes bug in kill task scheduling DruidCoordinatorSegmentKiller.findIntervalForKillTask()#2346
nishantmonu51 merged 2 commits intoapache:masterfrom
himanshug:fix_bug

Conversation

@himanshug
Copy link
Copy Markdown
Contributor

No description provided.

@himanshug himanshug added the Bug label Jan 27, 2016
@himanshug himanshug added this to the 0.9.0 milestone Jan 27, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 27, 2016

@himanshug this fails UT

@himanshug
Copy link
Copy Markdown
Contributor Author

@fjy fixed, my PR was not up-to-date post #2261 merge.

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 27, 2016

👍

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could this be replaced with return JodaUtils.umbrellaInterval(usedSegmentIntervals) ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nice tip, changed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The return contract on params is ill-defined. Some other methods modify the values in the params before returning them. Can you add a method javadoc describing any intended changes to params, or lack of changes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, added javadoc.

@drcrallen
Copy link
Copy Markdown
Contributor

👍 after travis

@nishantmonu51
Copy link
Copy Markdown
Member

👍

nishantmonu51 added a commit that referenced this pull request Jan 28, 2016
Fixes bug in kill task scheduling DruidCoordinatorSegmentKiller.findIntervalForKillTask()
@nishantmonu51 nishantmonu51 merged commit ef8e78c into apache:master Jan 28, 2016
@himanshug himanshug deleted the fix_bug branch February 8, 2016 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants