Introduce a MetricsFactory which can be used to produce metrics objects at the Table and Keyspace level, and which is customizable#2272
Open
scottfines wants to merge 12 commits intomainfrom
Open
Introduce a MetricsFactory which can be used to produce metrics objects at the Table and Keyspace level, and which is customizable#2272scottfines wants to merge 12 commits intomainfrom
scottfines wants to merge 12 commits intomainfrom
Conversation
Checklist before you submit for review
|
17cfece to
8cf72eb
Compare
…s from external systems
…verridden in other locations
…NoOpCassandraMetricsRegistry is used
|
❌ Build ds-cassandra-pr-gate/PR-2272 rejected by Butler6 regressions found Found 6 new test failuresFound 7 known test failures |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



This ensures that no JMX or other metrics are created for
KeyspaceandColumnFamilyStoreobjects that are opened without loading SSTables.Some highlights (and possible reasons why this solution won't work):
openSSTablesflag to determine whether or not metrics are collected. The reasoning being that "If the SSTables are loaded, then we probably don't care about the metrics either". However, that may not be true! In that case, we may want to modify this to include a distinct "metrics" flag, or explicitly pass in a metrics factory, or similar.KeyspaceMetricswithnull) is a less disruptive change on the surface, but other code throughout the system appears to implicitly depend on the metrics object not being null. Enough of that existed throughout the codebase that the choice became "guarantee that every possible usage allows for null entries" or "risk a NPE if I am wrong about whether or not this case will always be used correctly". I opted for the slightly more disruptive approach of creating a subclass that doesn't register metrics. If you are a person with more knowledge of the C* codebase and has more confident that the nullity solution will work, then I'm open to being convinced. But this approach seemed to be safer overall.