gerlowskija commented on code in PR #4514: URL: https://github.com/apache/solr/pull/4514#discussion_r3474918353
########## solr/core/src/java/org/apache/solr/util/circuitbreaker/TtlSampledMetric.java: ########## @@ -0,0 +1,109 @@ +/* + * 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 + * + * http://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.solr.util.circuitbreaker; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; + +/** + * Single-flight, time-bounded cache around an expensive metric sample. + * + * <ul> + * <li><b>Fresh:</b> within the TTL window every caller returns the cached value with one volatile + * read. + * <li><b>Stale:</b> exactly one thread runs the underlying sampler at a time (chosen by CAS); all + * other concurrent callers immediately return the most recent published value rather than + * queueing behind the refresh or piling onto the sampler. + * <li><b>Cold (no sample yet):</b> guarded by a one-time {@code synchronized (this)} block so + * only the first caller computes; the rest see the result. The monitor is held across the + * sampler invocation, so concurrent first-callers will block — but this path runs at most + * once per instance, and only callers that arrive before any value has been published are + * affected. + * </ul> + * + * <p>Used by {@link LoadAverageCircuitBreaker} and {@link MemoryCircuitBreaker} so that high-QPS + * admission control cannot stampede the OS load-average syscall or the post-GC heap-pool walk: even + * under thousands of concurrent {@code isTripped()} callers, the underlying sampler is invoked at + * most once per TTL window. + * + * <p><b>Exception behavior:</b> if the {@code source} supplier throws, the exception propagates to + * the calling thread and no new sample is published. Any previously-published value remains and + * other concurrent callers continue to see it; the next caller to find the entry stale will retry + * the supplier. The {@code refreshing} flag is always released, so a thrown sampler does not wedge + * the single-flight latch. + */ +final class TtlSampledMetric<T> { Review Comment: Yeah, it's small in that it's 50 LOC and no-dependencies. But even concise concurrency code is complex. If you've considered alternatives then I'm sure you've thought about it more than I have and I trust your judgement there. So, SGTM. Just wanted to make sure the tradeoff was called out 👍 -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
