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

SAMZA-2416 : Adding null-check before incrementing metrics for bytesSerialized #1234

Merged
merged 1 commit into from
Dec 10, 2019

Conversation

rmatharu-zz
Copy link
Contributor

Adding null-check before incrementing metrics for bytesSerialized

Symptom: User serde-serialization logic can choose to serialize non-null objects to a null, which implies delete the value. In this case, current code tries to increment the metrics using the size of the null and fails with an NPE.

Cause: Current code tries to increment the bytesSerialized metrics using the size of the null and fails with an NPE.

Fix: Added logic to skip metric value update if serde returns null.

Copy link
Contributor

@bkonold bkonold left a comment

Choose a reason for hiding this comment

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

More of a documentation question, but are there cases where samza-provided serde's will behave similarly? Or fail on execution when provided a null to serialize?

The change LGTM

@rmatharu-zz
Copy link
Contributor Author

@bkonold Samza provided serdes dont run into this, user-defined serdes though can map non-null objects to null.

@rmatharu-zz rmatharu-zz changed the title Adding null-check before incrementing metrics for bytesSerialized SAMZA-2416 : Adding null-check before incrementing metrics for bytesSerialized Dec 10, 2019
@rmatharu-zz rmatharu-zz merged commit 6ff4863 into apache:master Dec 10, 2019
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