Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2743#discussion_r199499960
--- Diff:
storm-server/src/main/java/org/apache/storm/metric/timed/TimerDecorated.java ---
@@ -0,0 +1,47 @@
+/**
+ * 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.storm.metric.timed;
+
+import com.codahale.metrics.Timer;
+
+import java.util.concurrent.atomic.AtomicReference;
+
+public interface TimerDecorated extends AutoCloseable {
+
+ boolean hasStopped();
+
+ default boolean hasStopped(final AtomicReference<Timer.Context>
timing) {
+ return timing.get() == null;
+ }
+
+ long stopTiming();
--- End diff --
I am a little concerned that we have both close and stopTiming, but if
anyone calls stopTiming and then close the assertion will be thrown. It is
like you have a method that you want everyone to implement, but never use.
Could you add some javadocs to this method in particular, but also to the
others as well so that developers understand how to use these methods, as it is
not the most obvious. It would also be nice if the javadocs could point to one
of the implementations as an example of how to extend this as a cross cutting
concern.
---
