gharris1727 commented on code in PR #16266:
URL: https://github.com/apache/kafka/pull/16266#discussion_r1636839783
##########
clients/src/main/java/org/apache/kafka/common/utils/SystemTime.java:
##########
@@ -21,10 +21,17 @@
import java.util.function.Supplier;
/**
- * A time implementation that uses the system clock and sleep call. Use
`Time.SYSTEM` instead of creating an instance
- * of this class.
+ * A time implementation that uses the system clock and sleep call.
+ * Every kafka process has a single instance of class <code>SystemTime</code>
that allows the process to interface
Review Comment:
nit: I think the mention of "process" here is more confusing than helpful.
If you want to mention this is a singleton, say "Use the singleton
`Time.SYSTEM`" you can do that, or just leave the javadoc as-is.
##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/SustainedConnectionWorker.java:
##########
@@ -60,7 +60,7 @@
public class SustainedConnectionWorker implements TaskWorker {
private static final Logger log =
LoggerFactory.getLogger(SustainedConnectionWorker.class);
- private static final SystemTime SYSTEM_TIME = new SystemTime();
+ private static final Time SYSTEM_TIME = Time.SYSTEM;
Review Comment:
I don't mean the local variables and instance variables `time`, those can
stay as-is, as they can be useful later if someone wants to replace the Time
instance for the whole test.
I meant this duplicate static final variable. Either it should be an
instance variable like the rest, or inlined.
--
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]
