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

make sure to remove obsolete pending requests #1068

Merged
merged 3 commits into from
Jun 14, 2016
Merged

Conversation

ssalinas
Copy link
Member

@ssalinas ssalinas commented Jun 7, 2016

If statements got switched around a bit back when I did the work for incremental deploys, we used to be removing these. No breakage caused, just annoying extra logging and some extra iterations in the scheduler.

/cc @stevenschlansker @tpetr

@tpetr
Copy link
Contributor

tpetr commented Jun 7, 2016

@ssalinas for posterity, could you link to the commit where these things got switched around?

@@ -2984,4 +2984,18 @@ public void testQueueMultipleOneOffs() {
Assert.assertEquals(2, taskManager.getPendingTaskIds().size());
}

@Test
public void testObsoletePendingRequestsRemoved() {
Copy link
Contributor

Choose a reason for hiding this comment

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

🏆

@ssalinas
Copy link
Member Author

ssalinas commented Jun 7, 2016

@ssalinas ssalinas merged commit cdfaf39 into master Jun 14, 2016
@ssalinas ssalinas deleted the obsolete_reqs branch June 14, 2016 13:14
@stevenschlansker
Copy link
Contributor

Any chance this is worth a patch release? All our masters fill their disks a couple of times a week right now.

@tpetr tpetr added this to the 0.8.0 milestone Jun 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants