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

Add a setting to skip null counters #1759

Merged
merged 1 commit into from
Dec 19, 2017
Merged

Conversation

johnynek
Copy link
Collaborator

We have observed NPEs for null counters at least two companies
using scalding. We have not root-caused this issue or found
a good fix, but previously set to ignore all null counters.

Since some people rely on counters this is not a great plan,
so instead this patch makes ignoring an opt-in behavior.

closes #1732

related to #1716 and #1726

cc @tdyas @rstewart this will change the default to error on null again. When you upgrade you will need to opt in to silently dropping counters using scalding.counters.skipnull=true

We have observed NPEs for null counters at least two companies
using scalding. We have not root-caused this issue or found
a good fix, but previously set to ignore all null counters.

Since some people rely on counters this is not a great plan,
so instead this patch makes ignoring an opt-in behavior.
@CLAassistant
Copy link

CLAassistant commented Dec 19, 2017

CLA assistant check
All committers have signed the CLA.

@ianoc
Copy link
Collaborator

ianoc commented Dec 19, 2017

lgtm

@ianoc ianoc merged commit 37c2de2 into develop Dec 19, 2017
@ianoc ianoc deleted the oscar/optional-unsafe-counters branch December 19, 2017 19:34
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.

Add an option to make jobs not fail is counters are null
3 participants