Skip to content

Comments

clearer errors and logging for limit-active-tasks#4208

Merged
ddadlani merged 1 commit intomasterfrom
cleanup-limit-active-tasks
Aug 1, 2019
Merged

clearer errors and logging for limit-active-tasks#4208
ddadlani merged 1 commit intomasterfrom
cleanup-limit-active-tasks

Conversation

@ddadlani
Copy link
Contributor

@ddadlani ddadlani commented Aug 1, 2019

As part of merging in changes from limit-active-tasks and refactoring the task step, we accidentally introduced an error in the logs where we tried to decrease the number of active tasks even if we were not using limit-active-tasks.

This PR fixes that, as well as cleaning up other logging and naming for better clarity.

Signed-off-by: Denise Yu [email protected]
Co-authored-by: Divya Dadlani [email protected]

Signed-off-by: Denise Yu <[email protected]>
Co-authored-by: Divya Dadlani <[email protected]>
@ddadlani ddadlani merged commit cb9e92d into master Aug 1, 2019
@ddadlani ddadlani deleted the cleanup-limit-active-tasks branch August 1, 2019 17:50
@aledegano
Copy link
Contributor

@ddadlani Thank you so much for improving this.
I have a question: when I originally implemented the decrease_active_task function I thought a lot about doing so within the lock or not.
In the end I couldn't find any good reason to do it serially so I didn't.
Can I ask you what do you anticipate the problem could be by not doing so serially?
Thanks!

@jamieklassen jamieklassen added the release/undocumented This didn't warrant being documented or put in release notes. label Aug 26, 2019
@jamieklassen jamieklassen added this to the v5.5.0 milestone Aug 26, 2019
jamieklassen pushed a commit that referenced this pull request Aug 26, 2019
#4118
#4148
#4208
#4277
#4142
#4221
#4293

Signed-off-by: James Thomson <[email protected]>
Co-authored-by: Jamie Klassen <[email protected]>
matthewpereira pushed a commit that referenced this pull request Sep 5, 2019
#4118
#4148
#4208
#4277
#4142
#4221
#4293

Signed-off-by: James Thomson <[email protected]>
Co-authored-by: Jamie Klassen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/undocumented This didn't warrant being documented or put in release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants