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

Allow RawMessage's payload to be empty (but not null) #1995

Merged
merged 2 commits into from Mar 30, 2016

Conversation

Projects
None yet
2 participants
@kroepke
Member

kroepke commented Mar 30, 2016

RawMessage previously threw an exception when the payload was empty during construction. This lead to errors in the call sites, which, depending on the input, could cause connections and/or threads to be closed, e.g. the Kafka consumer would be shut down.

With this change we accept empty payloads, but discard them before inserting them into the input buffer. Since this happens in a central place we should cover all available inputs.
We also track the number of discarded empty messages and show this with each input next to the throughput metrics. Empty payloads usually indicate a sender error but can be safely ignored for now. Discarded messages do not count towards the local or global incoming messages as they are discarded before being journaled and processed by Graylog.

This change also improved the exception logging in the NettyTransport, where we now are way less verbose when faced with a "Connection reset by peer". We don't currently write significant information back to the client so we don't need to log the entire (meaningless) stacktrace here. The information is still available on TRACE.

fix #1584
fix #1546

Allow RawMessage's payload to be empty (but not null)
RawMessage previously threw an exception when the payload was empty during construction. This lead to errors in the call sites, which, depending on the input, could cause connections and/or threads to be closed, e.g. the Kafka consumer would be shut down.

With this change we accept empty payloads, but discard them before inserting them into the input buffer. Since this happens in a central place we should cover all available inputs.
We also track the number of discarded empty messages and show this with each input next to the throughput metrics. Empty payloads usually indicate a sender error but can be safely ignored for now. Discarded messages do not count towards the local or global incoming messages as they are discarded before being journaled and processed by Graylog.

 This change also improved the exception logging in the NettyTransport, where we now are way less verbose when faced with a "Connection reset by peer". We don't currently write significant information back to the client so we don't need to log the entire (meaningless) stacktrace here. The information is still available on TRACE.

fix #1584

@kroepke kroepke added this to the 2.0.0 milestone Mar 30, 2016

@joschi joschi self-assigned this Mar 30, 2016

@joschi

This comment has been minimized.

Contributor

joschi commented Mar 30, 2016

LGTM. 👍

@joschi joschi merged commit d99c3d3 into master Mar 30, 2016

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci-server-integration Jenkins build graylog2-server-integration-pr 771 has succeeded
Details
ci-web-linter Jenkins build graylog-pr-linter-check 260 has succeeded
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@joschi joschi deleted the issue-1584 branch Mar 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment