METRON-1467: Replace guava caches in places where the keyspace might be large #947
Conversation
cestella
changed the title
|
I ran this up with vagrant and ensured:
|
cestella
changed the title
| loader = s -> new HashMap<>(); | ||
| cache = Caffeine.newBuilder().maximumSize(maxCacheSize) | ||
| .expireAfterWrite(maxTimeRetain, TimeUnit.MINUTES) | ||
| .removalListener(new JoinRemoveListener()) |
There was a problem hiding this comment.
It seems like we only want notified of a full cache when ERROR logging is set. Is that the case?
In the JoinRemoveListener we end up doing some work that we probably don't need to do unless ERROR logging is set. One easy fix would be to only add the "remove listener" if LOG.isDebugEnabled().
There was a problem hiding this comment.
So, I believe this was intentionally done before this PR (I migrated this to the new caching strategy) and the idea is that if a removal is happening from the join cache under specific circumstances, we want to know about it because a message could be being dropped because the cache is being overwhelmed. @merrimanr Can you chime in here on the rationale?
There was a problem hiding this comment.
Yes, it is pre-existing. We can address at a later time.
I remember now, maxing out this cache causes the Split/Join to fail, which is a major problem for the Split/Join topology. And this cache here is only for the Split/Join, not the Unified topology.
We should probably look at adding similar logging (only when ERROR enabled) for the other places where we use the cache. Or just some mechanism to periodically log cache stats. Anywho, down the road.
|
+1 LGTM |
Contributor Comments
Based on the performance tuning exercise as part of METRON-1460, guava has difficulties with cache sizes over 10k. We, unfortunately, are quite demanding of guava in this regard so we should transition a few uses of guava to Caffeine:
Test plan:
First, spin up full-dev and ensure things continue to work.
For myself, I created a new type called
dummy, added some stellar enrichments, stellar threat triage enrichments and a triage rule to ensure things continued to work with stellar (exercising the stellar processor cache and the enrichment bolt cache).Pull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our Development Guidelines for the complete guide to follow for contributions.
Please refer also to our Build Verification Guidelines for complete smoke testing guides.
In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
For all changes:
For code changes:
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?
Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
For documentation related changes:
Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via
site-book/target/site/index.html:Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that travis-ci is set up for your personal repository such that your branches are built there before submitting a pull request.