EdColeman commented on code in PR #3629: URL: https://github.com/apache/accumulo/pull/3629#discussion_r1267283060
########## core/src/main/java/org/apache/accumulo/core/util/cache/Caches.java: ########## @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.core.util.cache; + +import static com.google.common.base.Suppliers.memoize; + +import java.util.function.Function; +import java.util.function.Supplier; + +import org.apache.accumulo.core.metrics.MetricsProducer; +import org.apache.accumulo.core.metrics.MetricsUtil; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; + +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.binder.cache.CaffeineStatsCounter; + +public class Caches implements MetricsProducer { + + public enum CacheName { + BULK_IMPORT_FILE_LENGTHS, + CLASSLOADERS, + COMBINER_LOGGED_MSGS, + COMPACTIONS_COMPLETED, + COMPACTION_CONFIGS, + COMPACTION_DISPATCHERS, + COMPRESSION_ALGORITHM, + CRYPT_PASSWORDS, + HOST_REGEX_BALANCER_TABLE_REGEX, + HOSTING_REQUEST_CACHE, + INSTANCE_ID, Review Comment: Could instance, namespace and table ids share one cache that accepts AbstractId<>? They should be unique so that would not clash (I think, needs to be checked) And would it be possible to disable metrics for these, and maybe others? It seems like overkill to be tracking metrics for internal ids that provide limited or no value to the users. There does not seem to be any action that a user could take if, for example, the table id cache showed something. It may be better to have a builder that specifically required metrics to be enables, and defaults to none (or a null sink?). Having metrics automatically created every time a we create a cache for something seem we could be impacting performance of the metrics system for little or no benefit. Another use-case could be to allow metrics only during development - for example, it may be useful to see cache stats during development and testing to tune cache settings that are otherwise hard-coded. Not sure if that's practical or would ever be used. Something like an ENV switch that if PROD would be user facing metrics and DEV / TEST would be additional things that only apply when its likely that would be examined? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
