GitHub user nickwallen opened a pull request:
https://github.com/apache/metron/pull/1178
METRON-1757 Storm Profiler Serialization Exception
### Description
* When using Kryo serialization, specifically in Storm, the class must have
a no-arg constructor.
* The existing unit tests that validated Kryo serialization for the
Profiler classes, used the SerDeUtils class to perform Kryo serialization.
This class sets up Kryo in a slightly different way from how Storm does
internally. This made the tests not catch the missing no-arg constructor.
* The `ProfileMeasurementTest`'s setup method did not have the correct
annotation so it was never called. This caused the test to validate
serialization of a null, rather than an actual object.
* Removed the 'asm' dependency from 'metron-profiler-common'. Conflicting
version of this library was causing issues with the Kryo serialization tests.
There should be no dependency to Storm from `metron-profiler-common`.
### Work Around
If you are experiencing this issue, you can work around the problem by
changing the following values in your `profiler.properties`. This will cause
Storm to fall-back to Java serialization when Kryo serialization fails in Storm
for `org.apache.metron.common.configuration.profiler.ProfileResult`.
```
topology.fall.back.on.java.serialization=true
topology.kryo.register=[ org.apache.metron.profiler.ProfileMeasurement, \
org.apache.metron.profiler.ProfilePeriod, \
org.apache.metron.common.configuration.profiler.ProfileResultExpressions, \
org.apache.metron.common.configuration.profiler.ProfileTriageExpressions, \
org.apache.metron.common.configuration.profiler.ProfilerConfig, \
org.apache.metron.common.configuration.profiler.ProfileConfig, \
org.json.simple.JSONObject, \
org.json.simple.JSONArray, \
java.util.LinkedHashMap, \
org.apache.metron.statistics.OnlineStatisticsProvider ]
```
### Testing
1. Spin-up Full Dev
1. Create a profile by following the Profiler README as a guide.
1. Spin-up this change on a multi-node cluster. Run the profiler with
multiple workers. Create the same profile as before and ensure that values can
be persisted in HBase.
## Pull Request Checklist
- [ ] Is there a JIRA ticket associated with this PR? If not one needs to
be created at [Metron
Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [ ] Does your PR title start with METRON-XXXX where XXXX is the JIRA
number you are trying to resolve? Pay particular attention to the hyphen "-"
character.
- [ ] Has your PR been rebased against the latest commit within the target
branch (typically master)?
- [ ] Have you included steps to reproduce the behavior or problem that is
being changed or addressed?
- [ ] Have you included steps or a guide to how the change may be verified
and tested manually?
- [ ] Have you ensured that the full suite of tests and checks have been
executed in the root metron folder via:
- [ ] Have you written or updated unit tests and or integration tests to
verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] Have you verified the basic functionality of the build by building
and running locally with Vagrant full-dev environment or the equivalent?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nickwallen/metron METRON-1757
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/1178.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1178
----
commit 457aa3cf73506901a2b43ffcb2674bfacd2dd635
Author: Nick Allen <nick@...>
Date: 2018-08-28T18:12:01Z
metron-profiler-common should not depend on Storm
commit 3098270599d1f1cb24211fc981c1f116da2c85e4
Author: Nick Allen <nick@...>
Date: 2018-08-28T18:14:27Z
Serialization tests were not working as measurement always null. Also had
to exclude asm for tests to pass
commit d2d40bf0f7e9a9f7073126bab78b3cd001b40458
Author: Nick Allen <nick@...>
Date: 2018-08-28T19:47:13Z
Updated tests to catch bug with Kryo serialization
commit eccd79908c980b211a9c39848c1977de5be0b51b
Author: Nick Allen <nick@...>
Date: 2018-08-28T19:52:56Z
Added default constructor which is required by Kryo
commit a9ceb97b6d2167392225ee8768c69b33f788cff8
Author: Nick Allen <nick@...>
Date: 2018-08-28T20:06:10Z
Corrected comment
----
---
