From f78d40a604ee74baf54b4d9ce4c9f332ec1cc0cf Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Tue, 31 Oct 2017 21:23:10 -0400 Subject: [PATCH] Fix warnings/cleanup in alivenessmonitor Change-Id: I508ef8a4c54fbc2af4ae3af56755af73bf192dae Signed-off-by: Tom Pantelis --- .../protocols/test/AlivenessMonitorTest.java | 6 +- .../alivenessmonitor-impl/pom.xml | 7 + .../internal/AlivenessMonitor.java | 135 ++++++++---------- .../internal/AlivenessMonitorConstants.java | 32 ++--- .../internal/HwVtepTunnelsStateHandler.java | 40 ++++-- ...AlivenessMonitorAndProtocolsConstants.java | 5 +- .../AlivenessProtocolHandlerRegistry.java | 13 +- .../AlivenessProtocolHandlerRegistryImpl.java | 25 ++-- .../blueprint/alivenessmonitor.xml | 1 + 9 files changed, 135 insertions(+), 129 deletions(-) diff --git a/alivenessmonitor/alivenessmonitor-impl-protocols/src/test/java/org/opendaylight/controller/alivenessmonitor/protocols/test/AlivenessMonitorTest.java b/alivenessmonitor/alivenessmonitor-impl-protocols/src/test/java/org/opendaylight/controller/alivenessmonitor/protocols/test/AlivenessMonitorTest.java index f90a9f617..fd1fe7239 100644 --- a/alivenessmonitor/alivenessmonitor-impl-protocols/src/test/java/org/opendaylight/controller/alivenessmonitor/protocols/test/AlivenessMonitorTest.java +++ b/alivenessmonitor/alivenessmonitor-impl-protocols/src/test/java/org/opendaylight/controller/alivenessmonitor/protocols/test/AlivenessMonitorTest.java @@ -11,11 +11,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.argThat; -import static org.mockito.Mockito.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -170,7 +170,7 @@ public class AlivenessMonitorTest { AlivenessProtocolHandlerRegistry alivenessProtocolHandlerRegistry = new AlivenessProtocolHandlerRegistryImpl(); alivenessMonitor = new AlivenessMonitor(dataBroker, idManager, - notificationPublishService, notificationService, alivenessProtocolHandlerRegistry); + notificationPublishService, alivenessProtocolHandlerRegistry); arpHandler = new AlivenessProtocolHandlerARP(dataBroker, interfaceManager, alivenessProtocolHandlerRegistry, arpService); lldpHandler = new AlivenessProtocolHandlerLLDP(dataBroker, diff --git a/alivenessmonitor/alivenessmonitor-impl/pom.xml b/alivenessmonitor/alivenessmonitor-impl/pom.xml index 004ff0d15..03b337b6f 100644 --- a/alivenessmonitor/alivenessmonitor-impl/pom.xml +++ b/alivenessmonitor/alivenessmonitor-impl/pom.xml @@ -84,6 +84,13 @@ and is available at http://www.eclipse.org/legal/epl-v10.html org.apache.aries.blueprint blueprint-maven-plugin + + org.codehaus.mojo + findbugs-maven-plugin + + true + + diff --git a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/AlivenessMonitor.java b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/AlivenessMonitor.java index 735f08bcb..b8be57f42 100644 --- a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/AlivenessMonitor.java +++ b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/AlivenessMonitor.java @@ -26,7 +26,6 @@ import com.google.common.util.concurrent.JdkFutureAdapters; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.SettableFuture; -import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -39,7 +38,6 @@ import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.Semaphore; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; import javax.annotation.PreDestroy; @@ -50,7 +48,6 @@ import org.opendaylight.controller.liblldp.Packet; import org.opendaylight.controller.liblldp.PacketException; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService; -import org.opendaylight.controller.md.sal.binding.api.NotificationService; import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction; import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; @@ -110,7 +107,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.Pa import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.PacketProcessingListener; import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.PacketReceived; import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.SendToController; -import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.common.RpcError.ErrorType; @@ -132,20 +128,6 @@ public class AlivenessMonitor private static final boolean CREATE_MISSING_PARENT = true; private static final int INVALID_ID = 0; - private final DataBroker dataBroker; - private final ManagedNewTransactionRunner txRunner; - private final IdManagerService idManager; - private final NotificationPublishService notificationPublishService; - private final NotificationService notificationService; - private final AlivenessProtocolHandlerRegistry alivenessProtocolHandlerRegistry; - private final ConcurrentMap> monitoringTasks; - private final ScheduledExecutorService monitorService; - private final ExecutorService callbackExecutorService; - private final LoadingCache monitorIdKeyCache; - private final ListenerRegistration listenerRegistration; - // TODO clean up: visibility package local instead of private because accessed in HwVtepTunnelsStateHandler - final ConcurrentMap lockMap = new ConcurrentHashMap<>(); - private static class FutureCallbackImpl implements FutureCallback { private final String message; @@ -178,23 +160,31 @@ public class AlivenessMonitor } } + private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; + private final IdManagerService idManager; + private final NotificationPublishService notificationPublishService; + private final AlivenessProtocolHandlerRegistry alivenessProtocolHandlerRegistry; + private final ConcurrentMap> monitoringTasks = new ConcurrentHashMap<>(); + private final ScheduledExecutorService monitorService; + private final ExecutorService callbackExecutorService; + private final LoadingCache monitorIdKeyCache; + private final ConcurrentMap lockMap = new ConcurrentHashMap<>(); + @Inject public AlivenessMonitor(final DataBroker dataBroker, final IdManagerService idManager, final NotificationPublishService notificationPublishService, - final NotificationService notificationService, AlivenessProtocolHandlerRegistry alivenessProtocolHandlerRegistry) { this.dataBroker = dataBroker; this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); this.idManager = idManager; this.notificationPublishService = notificationPublishService; - this.notificationService = notificationService; this.alivenessProtocolHandlerRegistry = alivenessProtocolHandlerRegistry; monitorService = Executors.newScheduledThreadPool(THREAD_POOL_SIZE, ThreadFactoryProvider.builder().namePrefix("Aliveness Monitoring Task").logger(LOG).build().get()); callbackExecutorService = Executors.newFixedThreadPool(THREAD_POOL_SIZE, ThreadFactoryProvider.builder().namePrefix("Aliveness Callback Handler").logger(LOG).build().get()); - monitoringTasks = new ConcurrentHashMap<>(); createIdPool(); monitorIdKeyCache = CacheBuilder.newBuilder().build(new CacheLoader() { @@ -205,7 +195,6 @@ public class AlivenessMonitor } }); - listenerRegistration = notificationService.registerNotificationListener(this); LOG.info("{} started", getClass().getSimpleName()); } @@ -215,18 +204,11 @@ public class AlivenessMonitor monitorIdKeyCache.cleanUp(); monitorService.shutdown(); callbackExecutorService.shutdown(); - if (listenerRegistration != null) { - listenerRegistration.close(); - } LOG.info("{} close", getClass().getSimpleName()); } - private ThreadFactory getMonitoringThreadFactory(String threadNameFormat) { - ThreadFactoryBuilder builder = new ThreadFactoryBuilder(); - builder.setNameFormat(threadNameFormat); - builder.setUncaughtExceptionHandler( - (thread, ex) -> LOG.error("Received Uncaught Exception event in Thread: {}", thread.getName(), ex)); - return builder.build(); + Semaphore getLock(String key) { + return lockMap.get(key); } private void createIdPool() { @@ -234,8 +216,8 @@ public class AlivenessMonitor .setPoolName(AlivenessMonitorConstants.MONITOR_IDPOOL_NAME) .setLow(AlivenessMonitorConstants.MONITOR_IDPOOL_START) .setHigh(AlivenessMonitorConstants.MONITOR_IDPOOL_SIZE).build(); - Future> result = idManager.createIdPool(createPool); - Futures.addCallback(JdkFutureAdapters.listenInPoolThread(result), new FutureCallback>() { + Future> resultFuture = idManager.createIdPool(createPool); + Futures.addCallback(JdkFutureAdapters.listenInPoolThread(resultFuture), new FutureCallback>() { @Override public void onFailure(Throwable error) { @@ -243,14 +225,14 @@ public class AlivenessMonitor } @Override - public void onSuccess(RpcResult result) { + public void onSuccess(@Nonnull RpcResult result) { if (result.isSuccessful()) { LOG.debug("Created IdPool for Aliveness Monitor Service"); } else { LOG.error("RPC to create Idpool failed {}", result.getErrors()); } } - }); + }, callbackExecutorService); } private int getUniqueId(final String idKey) { @@ -310,7 +292,7 @@ public class AlivenessMonitor return; } - Object objPayload = packetInFormatted.getPayload(); + Packet objPayload = packetInFormatted.getPayload(); if (objPayload == null) { LOG.trace("Unsupported packet type. Ignoring the packet..."); @@ -321,8 +303,8 @@ public class AlivenessMonitor LOG.trace("onPacketReceived packet: {}, packet class: {}", packetReceived, objPayload.getClass()); } - AlivenessProtocolHandler livenessProtocolHandler = alivenessProtocolHandlerRegistry - .getOpt(objPayload.getClass()); + AlivenessProtocolHandler livenessProtocolHandler = alivenessProtocolHandlerRegistry + .getOpt(Packet.class); if (livenessProtocolHandler == null) { return; } @@ -355,7 +337,7 @@ public class AlivenessMonitor Futures.addCallback(stateResult, new FutureCallback>() { @Override - public void onSuccess(Optional optState) { + public void onSuccess(@Nonnull Optional optState) { if (optState.isPresent()) { final MonitoringState currentState = optState.get(); @@ -364,8 +346,8 @@ public class AlivenessMonitor LOG.trace("OnPacketReceived : Monitoring state from ODS : {} ", currentState); } - Long responsePendingCount = currentState.getResponsePendingCount(); - + // Long responsePendingCount = currentState.getResponsePendingCount(); + // // Need to relook at the pending count logic to support N // out of M scenarios // if (currentState.getState() != LivenessState.Up) { @@ -378,7 +360,7 @@ public class AlivenessMonitor // responsePendingCount = // currentState.getResponsePendingCount() - 1; // } - responsePendingCount = INITIAL_COUNT; + Long responsePendingCount = INITIAL_COUNT; final boolean stateChanged = currentState.getState() == LivenessState.Down || currentState.getState() == LivenessState.Unknown; @@ -415,7 +397,7 @@ public class AlivenessMonitor LOG.trace("Error in writing monitoring state: {} to Datastore", state); } } - }); + }, callbackExecutorService); } else { if (LOG.isTraceEnabled()) { LOG.trace("Monitoring State not available for key: {} to process the Packet received", @@ -435,7 +417,7 @@ public class AlivenessMonitor tx.cancel(); releaseLock(lock); } - }); + }, callbackExecutorService); } private String getIpAddress(EndpointType endpoint) { @@ -512,7 +494,7 @@ public class AlivenessMonitor String idKey = getUniqueKey(interfaceName, ethType.toString(), srcEndpointType, destEndpointType); final long monitorId = getUniqueId(idKey); Optional optKey = read(LogicalDatastoreType.OPERATIONAL, getMonitoringInfoId(monitorId)); - final AlivenessProtocolHandler handler; + final AlivenessProtocolHandler handler; if (optKey.isPresent()) { String message = String.format( "Monitoring for the interface %s with this configuration " + "is already registered.", @@ -572,7 +554,7 @@ public class AlivenessMonitor LOG.debug("Scheduling monitor task for config: {}", in); scheduleMonitoringTask(monitoringInfo, profile.getMonitorInterval()); } - }, MoreExecutors.directExecutor()); + }, callbackExecutorService); } associateMonitorIdWithInterface(monitorId, interfaceName); @@ -613,10 +595,11 @@ public class AlivenessMonitor CREATE_MISSING_PARENT); } return tx.submit(); - }); + }, callbackExecutorService); Futures.addCallback(updateFuture, new FutureCallbackImpl( - String.format("Association of monitorId %d with Interface %s", monitorId, interfaceName))); + String.format("Association of monitorId %d with Interface %s", monitorId, interfaceName)), + MoreExecutors.directExecutor()); } private void scheduleMonitoringTask(MonitoringInfo monitoringInfo, long monitorInterval) { @@ -648,7 +631,6 @@ public class AlivenessMonitor } @Override - @SuppressWarnings("resource") // tx.close() in Future's callback(s) public Future> monitorUnpause(MonitorUnpauseInput input) { LOG.debug("Monitor Unpause operation invoked for monitor id: {}", input.getMonitorId()); final SettableFuture> result = SettableFuture.create(); @@ -669,7 +651,7 @@ public class AlivenessMonitor } @Override - public void onSuccess(Optional optInfo) { + public void onSuccess(@Nonnull Optional optInfo) { if (optInfo.isPresent()) { final MonitoringInfo info = optInfo.get(); ListenableFuture> readProfile = tx.read(LogicalDatastoreType.OPERATIONAL, @@ -687,7 +669,7 @@ public class AlivenessMonitor } @Override - public void onSuccess(Optional optProfile) { + public void onSuccess(@Nonnull Optional optProfile) { tx.close(); if (optProfile.isPresent()) { updateMonitorStatusTo(monitorId, MonitorStatus.Started, @@ -712,7 +694,7 @@ public class AlivenessMonitor RpcResultBuilder.failed().withError(ErrorType.APPLICATION, msg).build()); } } - }); + }, callbackExecutorService); } else { tx.close(); String msg = String.format("Monitoring info associated with id %d is not present", monitorId); @@ -863,13 +845,14 @@ public class AlivenessMonitor "Monitoring State associated with id %d is not present to send packet out.", monitorId); return Futures.immediateFailedFuture(new RuntimeException(errorMsg)); } - }); + }, callbackExecutorService); Futures.addCallback(writeResult, new FutureCallback() { @Override public void onSuccess(Void noarg) { // invoke packetout on protocol handler - AlivenessProtocolHandler handler = alivenessProtocolHandlerRegistry.getOpt(profile.getProtocolType()); + AlivenessProtocolHandler handler = + alivenessProtocolHandlerRegistry.getOpt(profile.getProtocolType()); if (handler != null) { LOG.debug("Sending monitoring packet {}", monitoringInfo); handler.startMonitoringTask(monitoringInfo); @@ -883,7 +866,7 @@ public class AlivenessMonitor error); releaseLock(lock); } - }); + }, callbackExecutorService); } void publishNotification(final Long monitorId, final LivenessState state) { @@ -901,13 +884,13 @@ public class AlivenessMonitor public void onSuccess(Object arg) { LOG.trace("Successful in notifying listeners for id {} - state {}", monitorId, state); } - }); + }, callbackExecutorService); } @Override public Future> monitorProfileCreate(final MonitorProfileCreateInput input) { LOG.debug("Monitor Profile Create operation - {}", input.getProfile()); - final SettableFuture> result = SettableFuture.create(); + final SettableFuture> returnFuture = SettableFuture.create(); Profile profile = input.getProfile(); final Long failureThreshold = profile.getFailureThreshold(); final Long monitorInterval = profile.getMonitorInterval(); @@ -927,7 +910,7 @@ public class AlivenessMonitor .setProfileId(profileId).build(); String msg = String.format("Monitor profile %s already present for the given input", input); LOG.warn(msg); - result.set(RpcResultBuilder.success(output) + returnFuture.set(RpcResultBuilder.success(output) .withWarning(ErrorType.PROTOCOL, "profile-exists", msg).build()); } else { final MonitorProfile monitorProfile = new MonitorProfileBuilder().setId(profileId) @@ -941,7 +924,7 @@ public class AlivenessMonitor String msg = String.format("Error when storing monitorprofile %s in datastore", monitorProfile); LOG.error(msg, error); - result.set(RpcResultBuilder.failed() + returnFuture.set(RpcResultBuilder.failed() .withError(ErrorType.APPLICATION, msg, error).build()); } @@ -949,11 +932,11 @@ public class AlivenessMonitor public void onSuccess(Void noarg) { MonitorProfileCreateOutput output = new MonitorProfileCreateOutputBuilder() .setProfileId(profileId).build(); - result.set(RpcResultBuilder.success(output).build()); + returnFuture.set(RpcResultBuilder.success(output).build()); } - }); + }, callbackExecutorService); } - return result; + return returnFuture; }, callbackExecutorService); Futures.addCallback(resultFuture, new FutureCallback>() { @Override @@ -961,7 +944,7 @@ public class AlivenessMonitor // This would happen when any error happens during reading for // monitoring profile String msg = String.format("Error in creating monitorprofile - %s", input); - result.set(RpcResultBuilder.failed() + returnFuture.set(RpcResultBuilder.failed() .withError(ErrorType.APPLICATION, msg, error).build()); LOG.error(msg, error); } @@ -971,7 +954,7 @@ public class AlivenessMonitor LOG.debug("Successfully created monitor Profile {} ", input); } }, callbackExecutorService); - return result; + return returnFuture; } @Override @@ -1037,7 +1020,7 @@ public class AlivenessMonitor releaseId(id); result.set(RpcResultBuilder.success().build()); } - }); + }, callbackExecutorService); } else { String msg = String.format("Monitor profile with Id: %d does not exist", profileId); LOG.info(msg); @@ -1085,7 +1068,8 @@ public class AlivenessMonitor } tx.delete(LogicalDatastoreType.OPERATIONAL, getMonitoringInfoId(monitorId)); - }), new FutureCallbackImpl(String.format("Delete monitor state with Id %d", monitorId))); + }), new FutureCallbackImpl(String.format("Delete monitor state with Id %d", monitorId)), + MoreExecutors.directExecutor()); MonitoringInfo info = optInfo.get(); String interfaceName = getInterfaceName(info.getSource().getEndpointType()); @@ -1128,10 +1112,11 @@ public class AlivenessMonitor tx.cancel(); return Futures.immediateFuture(null); } - }); + }, MoreExecutors.directExecutor()); Futures.addCallback(updateFuture, new FutureCallbackImpl( - String.format("Dis-association of monitorId %d with Interface %s", monitorId, interfaceName))); + String.format("Dis-association of monitorId %d with Interface %s", monitorId, interfaceName)), + MoreExecutors.directExecutor()); } private void releaseIdForMonitoringInfo(MonitoringInfo info) { @@ -1197,13 +1182,13 @@ public class AlivenessMonitor newStatus, monitorId); } return tx.submit(); - }); + }, MoreExecutors.directExecutor()); Futures.addCallback(writeResult, new FutureCallbackImpl( - String.format("Monitor status update for %d to %s", monitorId, newStatus.toString()))); + String.format("Monitor status update for %d to %s", monitorId, newStatus.toString())), + MoreExecutors.directExecutor()); } - @SuppressWarnings("resource") // tx.close() in Future's callback(s) private void resumeMonitoring(final long monitorId) { final ReadOnlyTransaction tx = dataBroker.newReadOnlyTransaction(); ListenableFuture> readInfoResult = tx.read(LogicalDatastoreType.OPERATIONAL, @@ -1219,7 +1204,7 @@ public class AlivenessMonitor } @Override - public void onSuccess(Optional optInfo) { + public void onSuccess(@Nonnull Optional optInfo) { if (optInfo.isPresent()) { final MonitoringInfo info = optInfo.get(); ListenableFuture> readProfile = tx.read(LogicalDatastoreType.OPERATIONAL, @@ -1235,7 +1220,7 @@ public class AlivenessMonitor } @Override - public void onSuccess(Optional optProfile) { + public void onSuccess(@Nonnull Optional optProfile) { tx.close(); if (optProfile.isPresent()) { updateMonitorStatusTo(monitorId, MonitorStatus.Started, @@ -1249,14 +1234,14 @@ public class AlivenessMonitor LOG.warn("Monitor resume Failed. {}", msg); } } - }); + }, MoreExecutors.directExecutor()); } else { tx.close(); String msg = String.format("Monitoring info associated with id %d is not present", monitorId); LOG.warn("Monitor resume Failed. {}", msg); } } - }); + }, MoreExecutors.directExecutor()); } // DATA STORE OPERATIONS diff --git a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/AlivenessMonitorConstants.java b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/AlivenessMonitorConstants.java index a4f4713f4..07a08a488 100644 --- a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/AlivenessMonitorConstants.java +++ b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/AlivenessMonitorConstants.java @@ -12,26 +12,26 @@ import org.opendaylight.genius.alivenessmonitor.protocols.AlivenessMonitorAndPro /** * Constants private to alivenessmonitor.internal. */ -public class AlivenessMonitorConstants { - static final String MONITOR_IDPOOL_NAME = "aliveness-monitor"; - static final long MONITOR_IDPOOL_START = 1L; - static final long MONITOR_IDPOOL_SIZE = 65535; - static final String SEPERATOR = AlivenessMonitorAndProtocolsConstants.SEPERATOR; +public interface AlivenessMonitorConstants { + String MONITOR_IDPOOL_NAME = "aliveness-monitor"; + long MONITOR_IDPOOL_START = 1L; + long MONITOR_IDPOOL_SIZE = 65535; + String SEPERATOR = AlivenessMonitorAndProtocolsConstants.SEPERATOR; // BFD parameters - static final String BFD_PARAM_ENABLE = "enable"; - static final String BFD_PARAM_MIN_RX = "min_rx"; - static final String BFD_PARAM_MIN_TX = "min_tx"; - static final String BFD_PARAM_DECAY_MIN_RX = "decay_min_rx"; - static final String BFD_PARAM_FORWARDING_IF_RX = "forwarding_if_rx"; - static final String BFD_PARAM_CPATH_DOWN = "cpath_down"; - static final String BFD_PARAM_CHECK_TNL_KEY = "check_tnl_key"; + String BFD_PARAM_ENABLE = "enable"; + String BFD_PARAM_MIN_RX = "min_rx"; + String BFD_PARAM_MIN_TX = "min_tx"; + String BFD_PARAM_DECAY_MIN_RX = "decay_min_rx"; + String BFD_PARAM_FORWARDING_IF_RX = "forwarding_if_rx"; + String BFD_PARAM_CPATH_DOWN = "cpath_down"; + String BFD_PARAM_CHECK_TNL_KEY = "check_tnl_key"; // BFD Local/Remote Configuration parameters - static final String BFD_CONFIG_BFD_DST_MAC = "bfd_dst_mac"; - static final String BFD_CONFIG_BFD_DST_IP = "bfd_dst_ip"; + String BFD_CONFIG_BFD_DST_MAC = "bfd_dst_mac"; + String BFD_CONFIG_BFD_DST_IP = "bfd_dst_ip"; - static final String BFD_OP_STATE = "state"; - static final String BFD_STATE_UP = "up"; + String BFD_OP_STATE = "state"; + String BFD_STATE_UP = "up"; } diff --git a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/HwVtepTunnelsStateHandler.java b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/HwVtepTunnelsStateHandler.java index 0fffe9ff9..00d9ef790 100644 --- a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/HwVtepTunnelsStateHandler.java +++ b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/internal/HwVtepTunnelsStateHandler.java @@ -13,10 +13,13 @@ import com.google.common.base.Optional; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.concurrent.Semaphore; +import javax.annotation.Nonnull; import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.controller.liblldp.Packet; @@ -36,7 +39,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.alivenessmonitor.rev import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.alivenessmonitor.rev160411.monitoring.states.MonitoringState; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.alivenessmonitor.rev160411.monitoring.states.MonitoringStateBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.PacketReceived; -import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.HwvtepPhysicalLocatorRef; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.PhysicalSwitchAugmentation; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.hwvtep.physical._switch.attributes.Tunnels; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.hwvtep.rev150901.hwvtep.physical._switch.attributes.TunnelsBuilder; @@ -64,7 +66,7 @@ import org.slf4j.LoggerFactory; @Singleton public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListener - implements AlivenessProtocolHandler, AutoCloseable { + implements AlivenessProtocolHandler { private static final Logger LOG = LoggerFactory.getLogger(HwVtepTunnelsStateHandler.class); private final DataBroker dataBroker; @@ -114,7 +116,7 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe final String monitorKey = getBfdMonitorKey(interfaceName); LOG.debug("Processing monitorKey: {} for received Tunnels update DCN", monitorKey); - final Semaphore lock = alivenessMonitor.lockMap.get(monitorKey); + final Semaphore lock = alivenessMonitor.getLock(monitorKey); LOG.debug("Acquiring lock for monitor key : {} to process monitor DCN", monitorKey); alivenessMonitor.acquireLock(lock); @@ -125,7 +127,7 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe Futures.addCallback(stateResult, new FutureCallback>() { @Override - public void onSuccess(Optional optState) { + public void onSuccess(@Nonnull Optional optState) { if (optState.isPresent()) { final MonitoringState currentState = optState.get(); if (currentState.getState() == newTunnelOpState) { @@ -162,7 +164,7 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe LOG.trace("Error in writing monitoring state: {} to Datastore", state); } } - }); + }, MoreExecutors.directExecutor()); } else { LOG.warn("Monitoring State not available for key: {} to process the Packet received", monitorKey); // Complete the transaction @@ -179,7 +181,7 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe tx.cancel(); alivenessMonitor.releaseLock(lock); } - }); + }, MoreExecutors.directExecutor()); } private LivenessState getTunnelOpState(List tunnelBfdStatus) { @@ -208,8 +210,8 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe } @Override - public Class getPacketClass() { - return null; + public Class getPacketClass() { + return Packet.class; } @Override @@ -217,10 +219,16 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe return null; } + // tunnelKey, nodeId, topologyId are initialized to null and immediately passed to getTunnelIdentifier which + // FindBugs as a "Load of known null value" violation. Not sure sure what the intent... + @SuppressFBWarnings("NP_LOAD_OF_KNOWN_NULL_VALUE") void resetMonitoringTask(boolean isEnable) { // TODO: get the corresponding hwvtep tunnel from the sourceInterface // once InterfaceMgr // implments renderer for hwvtep vxlan tunnels + + // tunnelKey, nodeId, topologyId are initialized to null and immediately passed to getTunnelIdentifier which + // FindBugs flags as a "Load of known null value" violation. Not sure sure what the intent... TunnelsKey tunnelKey = null; String nodeId = null; String topologyId = null; @@ -234,7 +242,9 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe List tunnelBfdParams = tunnel.getBfdParams(); if (tunnelBfdParams == null || tunnelBfdParams.isEmpty()) { LOG.debug("there is no bfd params available for the tunnel {}", tunnel); + return; } + Iterator tunnelBfdParamsInterator = tunnelBfdParams.iterator(); while (tunnelBfdParamsInterator.hasNext()) { BfdParams bfdParam = tunnelBfdParamsInterator.next(); @@ -275,15 +285,16 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe String tunnelLocalMacAddress = ""; String tunnelLocalIpAddress = ""; String tunnelRemoteMacAddress = ""; - String tunnelRemoteIpAddress = ""; List bfdParams = new ArrayList<>(); fillBfdParams(bfdParams, profile); List bfdLocalConfigs = new ArrayList<>(); fillBfdLocalConfigs(bfdLocalConfigs, tunnelLocalMacAddress, tunnelLocalIpAddress); List bfdRemoteConfigs = new ArrayList<>(); fillBfdRemoteConfigs(bfdRemoteConfigs, tunnelRemoteMacAddress); - TunnelsKey tunnelKey = null; - Tunnels tunnelWithBfd = new TunnelsBuilder().setKey(tunnelKey).setBfdParams(bfdParams) + // tunnelKey is initialized to null and passed to setKey which FindBugs flags as a + // "Load of known null value" violation. Not sure sure what the intent is... + //TunnelsKey tunnelKey = null; + Tunnels tunnelWithBfd = new TunnelsBuilder().setKey(/*tunnelKey*/ null).setBfdParams(bfdParams) .setBfdLocalConfigs(bfdLocalConfigs).setBfdRemoteConfigs(bfdRemoteConfigs).build(); // TODO: get the following parameters from the interface and use it to // update hwvtep datastore @@ -292,12 +303,13 @@ public class HwVtepTunnelsStateHandler extends HwvtepAbstractDataTreeChangeListe // into hwvtep datastore. if tunnels are not created during that time, // then start monitoring has to // be done as part of tunnel add DCN handling. - HwvtepPhysicalLocatorRef remoteRef = null; - HwvtepPhysicalLocatorRef localRef = null; +// HwvtepPhysicalLocatorRef remoteRef = null; +// HwvtepPhysicalLocatorRef localRef = null; String topologyId = ""; String nodeId = ""; MDSALUtil.syncUpdate(dataBroker, LogicalDatastoreType.CONFIGURATION, - getTunnelIdentifier(topologyId, nodeId, new TunnelsKey(localRef, remoteRef)), tunnelWithBfd); + getTunnelIdentifier(topologyId, nodeId, new TunnelsKey(/*localRef*/ null, /*remoteRef*/ null)), + tunnelWithBfd); } private void fillBfdRemoteConfigs(List bfdRemoteConfigs, String tunnelRemoteMacAddress) { diff --git a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/AlivenessMonitorAndProtocolsConstants.java b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/AlivenessMonitorAndProtocolsConstants.java index 12c8b3187..34bb6c7e0 100644 --- a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/AlivenessMonitorAndProtocolsConstants.java +++ b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/AlivenessMonitorAndProtocolsConstants.java @@ -10,8 +10,7 @@ package org.opendaylight.genius.alivenessmonitor.protocols; /** * Constants shared between alivenessmonitor.internal and alivenessmonitor.protocols.internal. */ -public class AlivenessMonitorAndProtocolsConstants { - - public static final String SEPERATOR = "."; +public interface AlivenessMonitorAndProtocolsConstants { + String SEPERATOR = "."; } diff --git a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/AlivenessProtocolHandlerRegistry.java b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/AlivenessProtocolHandlerRegistry.java index 1761f3efb..9b5d4304a 100644 --- a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/AlivenessProtocolHandlerRegistry.java +++ b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/AlivenessProtocolHandlerRegistry.java @@ -7,9 +7,10 @@ */ package org.opendaylight.genius.alivenessmonitor.protocols; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; -import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; +import org.opendaylight.controller.liblldp.Packet; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.alivenessmonitor.rev160411.EtherTypes; /** @@ -20,11 +21,11 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.alivenessmonitor.rev @ThreadSafe public interface AlivenessProtocolHandlerRegistry { - void register(EtherTypes etherType, AlivenessProtocolHandler protocolHandler); + void register(EtherTypes etherType, AlivenessProtocolHandler protocolHandler); - @Nullable AlivenessProtocolHandler getOpt(EtherTypes etherType); + @Nullable AlivenessProtocolHandler getOpt(EtherTypes etherType); - @Nullable AlivenessProtocolHandler getOpt(Class packetClass); + @Nullable AlivenessProtocolHandler getOpt(Class packetClass); - @NonNull AlivenessProtocolHandler get(EtherTypes etherType); + @Nonnull AlivenessProtocolHandler get(EtherTypes etherType); } diff --git a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/internal/AlivenessProtocolHandlerRegistryImpl.java b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/internal/AlivenessProtocolHandlerRegistryImpl.java index 091311210..a60a60b8a 100644 --- a/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/internal/AlivenessProtocolHandlerRegistryImpl.java +++ b/alivenessmonitor/alivenessmonitor-impl/src/main/java/org/opendaylight/genius/alivenessmonitor/protocols/internal/AlivenessProtocolHandlerRegistryImpl.java @@ -12,8 +12,7 @@ import java.util.HashMap; import java.util.Map; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Singleton; -import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; +import org.opendaylight.controller.liblldp.Packet; import org.opendaylight.genius.alivenessmonitor.protocols.AlivenessProtocolHandler; import org.opendaylight.genius.alivenessmonitor.protocols.AlivenessProtocolHandlerRegistry; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.alivenessmonitor.rev160411.EtherTypes; @@ -27,36 +26,38 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.alivenessmonitor.rev @ThreadSafe public class AlivenessProtocolHandlerRegistryImpl implements AlivenessProtocolHandlerRegistry { - private final Map ethTypeToProtocolHandler = new EnumMap<>(EtherTypes.class); - private final Map, AlivenessProtocolHandler> packetTypeToProtocolHandler = new HashMap<>(); + private final Map> ethTypeToProtocolHandler = + new EnumMap<>(EtherTypes.class); + private final Map, AlivenessProtocolHandler> packetTypeToProtocolHandler = new HashMap<>(); @Override - public synchronized void register(EtherTypes etherType, AlivenessProtocolHandler protocolHandler) { + public synchronized void register(EtherTypes etherType, AlivenessProtocolHandler protocolHandler) { ethTypeToProtocolHandler.put(etherType, protocolHandler); packetTypeToProtocolHandler.put(protocolHandler.getPacketClass(), protocolHandler); } @Override - public synchronized @Nullable AlivenessProtocolHandler getOpt(EtherTypes etherType) { + public synchronized AlivenessProtocolHandler getOpt(EtherTypes etherType) { return ethTypeToProtocolHandler.get(etherType); } + @SuppressWarnings("unchecked") @Override - public synchronized @Nullable AlivenessProtocolHandler getOpt(Class packetClass) { - return packetTypeToProtocolHandler.get(packetClass); + public synchronized AlivenessProtocolHandler getOpt(Class packetClass) { + return (AlivenessProtocolHandler) packetTypeToProtocolHandler.get(packetClass); } @Override - public @NonNull AlivenessProtocolHandler get(EtherTypes etherType) { - AlivenessProtocolHandler handler = getOpt(etherType); + public AlivenessProtocolHandler get(EtherTypes etherType) { + AlivenessProtocolHandler handler = getOpt(etherType); if (handler == null) { throw new IllegalStateException("No handler registered for etherType: " + etherType); } return handler; } - public @NonNull AlivenessProtocolHandler get(Class packetClass) { - AlivenessProtocolHandler handler = getOpt(packetClass); + public AlivenessProtocolHandler get(Class packetClass) { + AlivenessProtocolHandler handler = packetTypeToProtocolHandler.get(packetClass); if (handler == null) { throw new IllegalStateException("No handler registered for packetClass: " + packetClass); } diff --git a/alivenessmonitor/alivenessmonitor-impl/src/main/resources/org/opendaylight/blueprint/alivenessmonitor.xml b/alivenessmonitor/alivenessmonitor-impl/src/main/resources/org/opendaylight/blueprint/alivenessmonitor.xml index 8de6f166b..d0c9b94e1 100644 --- a/alivenessmonitor/alivenessmonitor-impl/src/main/resources/org/opendaylight/blueprint/alivenessmonitor.xml +++ b/alivenessmonitor/alivenessmonitor-impl/src/main/resources/org/opendaylight/blueprint/alivenessmonitor.xml @@ -27,5 +27,6 @@ and is available at http://www.eclipse.org/legal/epl-v10.html interface="org.opendaylight.yang.gen.v1.urn.opendaylight.packet.service.rev130709.PacketProcessingService" /> + -- 2.36.6