cshannon commented on code in PR #1726:
URL: https://github.com/apache/activemq/pull/1726#discussion_r3597050562
##########
activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java:
##########
@@ -372,6 +375,7 @@ public void run() {
// Once we have all required broker info we can attempt to
start
// the local and then remote sides of the bridge.
doStartLocalAndRemoteBridges();
+ startedTimestamp.set(System.currentTimeMillis());
Review Comment:
I would move this to the start() method just because the other call to set
back to 0 is in the stop() method so it makes more sense. I'd also add it to a
try/finally probably.
[Here](https://github.com/apache/activemq/blob/5dcc72b779c3b51b32bbad3daa6b44a7f19648b4/activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java#L262)
is what i mean.
Something like:
```java
try {
triggerStartAsyncNetworkBridgeCreation();
} catch (IOException e) {
LOG.warn("Caught exception from remote start", e);
} finally {
startedTimestamp.set(System.currentTimeMillis());
}
```
##########
activemq-broker/src/main/java/org/apache/activemq/network/NetworkConnector.java:
##########
@@ -164,22 +172,35 @@ public static ActiveMQDestination[]
getDurableTopicDestinations(final Set<Active
@Override
public void start() throws Exception {
serviceSupport.start();
+ startedTimestamp.set(System.currentTimeMillis());
+ stoppedTimestamp.set(0l);
}
@Override
public void stop() throws Exception {
serviceSupport.stop();
+ stoppedTimestamp.set(System.currentTimeMillis());
+ startedTimestamp.set(0l);
}
protected void handleStart() throws Exception {
if (localURI == null) {
throw new IllegalStateException("You must configure the 'localURI'
property");
}
LOG.info("Network Connector {} started", this);
+ bridgeExceptionCounter.setEnabled(true);
Review Comment:
Shouldn't this not be true by default? I thought network statistics were
optional and configurable?
##########
activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java:
##########
@@ -171,6 +172,7 @@ public abstract class DemandForwardingBridgeSupport
implements NetworkBridge, Br
private final ExecutorService syncExecutor =
Executors.newSingleThreadExecutor();
private Transport duplexInboundLocalBroker = null;
private ProducerInfo duplexInboundLocalProducerInfo;
+ private AtomicLong startedTimestamp = new AtomicLong(0L);
Review Comment:
```suggestion
private final AtomicLong startedTimestamp = new AtomicLong(0L);
```
##########
activemq-unit-tests/src/test/java/org/apache/activemq/bugs/AMQ4160Test.java:
##########
@@ -351,6 +366,7 @@ public ObjectName getMbeanObjectName() {
public void resetStats(){
next.resetStats();
}
+
Review Comment:
nit: remove extra line
--
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]
For further information, visit: https://activemq.apache.org/contact
