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

Fix another regression: a crash introduced in commit feb892f #82

Merged
merged 1 commit into from
Dec 10, 2015

Conversation

erikdw
Copy link
Collaborator

@erikdw erikdw commented Dec 10, 2015

Commit feb892f (from PR #79) introduced a NullPointerException crash:

After the first pass through the computeResourcesForSlot() main loop, the
executorCpuResources and executorMemResources variables are not populated,
and thus incorreclty null, because subtractedExecutorResources was set to
true on the 1st pass.

The fix is to unconditionally set them to the requested resource amounts
unconditionally, and only subtract the resources out of the offer on the
1st pass.

Also an unrelated change: clarify that the reservations that are being skipped
in a couple functions are dynamic reservations.

@erikdw
Copy link
Collaborator Author

erikdw commented Dec 10, 2015

@JessicaLHartog: please review.

@erikdw erikdw changed the title Fix another regression: a crash introduced in #78 (feb892f) Fix another regression: a crash introduced in feb892f Dec 10, 2015
Commit feb892f (from PR mesos#79) introduced a NullPointerException crash:

After the first pass through the `computeResourcesForSlot()` main loop, the
`executorCpuResources` and `executorMemResources` variables are not populated,
and thus incorreclty null, because `subtractedExecutorResources ` was set to
true on the 1st pass.

The fix is to unconditionally set them to the requested resource amounts
unconditionally, and only subtract the resources out of the offer on the
1st pass.

Also an unrelated change:  clarify that the reservations that are being skipped
in a couple functions are *dynamic* reservations.
@erikdw erikdw changed the title Fix another regression: a crash introduced in feb892f Fix another regression: a crash introduced in commit feb892f Dec 10, 2015
@JessicaLHartog
Copy link
Collaborator

LGTM 👍

@erikdw
Copy link
Collaborator Author

erikdw commented Dec 10, 2015

Here goes nothing! ;-) Hope this works well for everyone. Definitely need to fix #81

erikdw added a commit that referenced this pull request Dec 10, 2015
Fix another regression: a crash introduced in commit feb892f
@erikdw erikdw merged commit dfbebd8 into mesos:master Dec 10, 2015
@erikdw erikdw deleted the fix-npe branch December 10, 2015 20:35
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.

2 participants