Add (remote) cache metrics#3033
Conversation
44ba793 to
d61cc72
Compare
jicelhay
left a comment
There was a problem hiding this comment.
@jicelhay reviewed 4 files and all commit messages, and made 2 comments.
Reviewable status: 4 of 7 files reviewed, 2 unresolved discussions (waiting on gbrodman and ptkach).
core/src/main/java/google/registry/cache/CacheMetrics.java line 38 at r1 (raw file):
ImmutableSet.of( LabelDescriptor.create("cache_name", "The type of the cache (domain/host)."), LabelDescriptor.create("hit_type", "The type of cache hit, if the object was found."));
"if the object was found" I don't think that description is accurate if you're trackig misses in the label, right?
core/src/main/java/google/registry/cache/MultilayerEppResourceCache.java line 72 at r1 (raw file):
possibleValue = loadFromDatabase(key); if (possibleValue.isEmpty()) { cacheMetrics.recordLookup(clazz.getSimpleName(), CacheMetrics.CacheHitType.MISS_NONEXISTENT);
What's the reason to track MISS_NONEXISTENT?
This only applies to the CacheModule-provided caches because we don't want to have to deal with all the various other caches. We'll want to know the various ratios between types of cache hits/misses when evaluating the usefulness of the remote caching.
gbrodman
left a comment
There was a problem hiding this comment.
@gbrodman made 2 comments.
Reviewable status: 4 of 7 files reviewed, 2 unresolved discussions (waiting on jicelhay and ptkach).
core/src/main/java/google/registry/cache/CacheMetrics.java line 38 at r1 (raw file):
Previously, jicelhay (Juan Celhay) wrote…
"if the object was found" I don't think that description is accurate if you're trackig misses in the label, right?
Done
core/src/main/java/google/registry/cache/MultilayerEppResourceCache.java line 72 at r1 (raw file):
Previously, jicelhay (Juan Celhay) wrote…
What's the reason to track MISS_NONEXISTENT?
Requests for domains that don't exist will always fall through to the database. It looks like for now these aren't a huge portion of requests (maybe 15-20%?) but if that percentage goes up, the caches won't be of as much use and the database will still get hammered.
In that case we will need to come up with different solutions, which could include maybe caching nonexistence or using some complicated system of nested bloom filters.
jicelhay
left a comment
There was a problem hiding this comment.
@jicelhay reviewed 4 files and all commit messages, and resolved 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ptkach).
This only applies to the CacheModule-provided caches because we don't want to have to deal with all the various other caches. We'll want to know the various ratios between types of cache hits/misses when evaluating the usefulness of the remote caching.
This change is