This is a hot fix for a race condition issue which can happen during a service lifecycle.
An application which manages services in not aware when the unregistration is finished, hence
service close is followed by immendiate service registration which may fail consequently.
This condition is handled by reatempting service regsiration after 10 ms.
Also fixes RIBImpl - create TX chain in #instantiateServiceInstance and asynchronously clean operatoinal DS.
Change-Id: I110badc8bfb482a5b7cb6545b882cda2d0c661ae
Signed-off-by: Milos Fabian <milfabia@cisco.com>
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.CheckedFuture;
import com.google.common.util.concurrent.ListenableFuture;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import org.opendaylight.protocol.bgp.rib.spi.ExportPolicyPeerTracker;
import org.opendaylight.protocol.bgp.rib.spi.RIBExtensionConsumerContext;
import org.opendaylight.protocol.bgp.rib.spi.RibSupportUtils;
+import org.opendaylight.protocol.bgp.rib.spi.util.ClusterSingletonServiceRegistrationHelper;
import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.AsNumber;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev130919.BgpTableType;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.BgpRib;
private static final Logger LOG = LoggerFactory.getLogger(RIBImpl.class);
private static final QName RIB_ID_QNAME = QName.create(Rib.QNAME, "id").intern();
private static final ContainerNode EMPTY_TABLE_ATTRIBUTES = ImmutableNodes.containerNode(org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.tables.Attributes.QNAME);
+ private static final int MAX_REGISTRATION_ATTEMPTS = 10;
+ private static final int SLEEP_TIME = MAX_REGISTRATION_ATTEMPTS;
private final BGPDispatcher dispatcher;
- private final DOMTransactionChain domChain;
private final AsNumber localAs;
private final BgpId bgpIdentifier;
private final Set<BgpTableType> localTables;
private final RibId ribId;
private final Map<TablesKey, ExportPolicyPeerTracker> exportPolicyPeerTrackerMap;
+ private DOMTransactionChain domChain;
+
public RIBImpl(final ClusterSingletonServiceProvider provider, final RibId ribId, final AsNumber localAs, final BgpId localBgpId,
final ClusterIdentifier clusterId, final RIBExtensionConsumerContext extensions, final BGPDispatcher dispatcher,
final BindingCodecTreeFactory codecFactory, final DOMDataBroker domDataBroker, final List<BgpTableType> localTables,
final BgpDeployer.WriteConfiguration configurationWriter) {
super(InstanceIdentifier.create(BgpRib.class).child(Rib.class, new RibKey(Preconditions.checkNotNull(ribId))));
- this.domChain = domDataBroker.createTransactionChain(this);
this.localAs = Preconditions.checkNotNull(localAs);
this.bgpIdentifier = Preconditions.checkNotNull(localBgpId);
this.dispatcher = Preconditions.checkNotNull(dispatcher);
this.registration.close();
this.registration = null;
}
- if(this.domChain != null) {
- this.domChain.close();
- }
}
@Override
@Override
public void instantiateServiceInstance() {
+ this.domChain = this.domDataBroker.createTransactionChain(this);
if(this.configurationWriter != null) {
this.configurationWriter.apply();
}
@Override
public ListenableFuture<Void> closeServiceInstance() {
LOG.info("Close RIB Singleton Service {}", getIdentifier());
- for (final LocRibWriter locRib : this.locRibs) {
- try {
- locRib.close();
- } catch (final Exception e) {
- LOG.warn("Could not close LocalRib reference: {}", locRib, e);
- }
- }
- try {
- final DOMDataWriteTransaction t = this.domChain.newWriteOnlyTransaction();
- t.delete(LogicalDatastoreType.OPERATIONAL, getYangRibId());
- t.submit().checkedGet();
- } catch (final TransactionCommitFailedException e) {
- LOG.warn("Failed to remove RIB instance {} from DS.", getYangRibId(), e);
- }
+ this.locRibs.forEach(LocRibWriter::close);
+ this.locRibs.clear();
+
this.renderStats.getLocRibRouteCounter().resetAll();
- return Futures.immediateFuture(null);
+
+ final DOMDataWriteTransaction t = this.domChain.newWriteOnlyTransaction();
+ t.delete(LogicalDatastoreType.OPERATIONAL, getYangRibId());
+ final CheckedFuture<Void, TransactionCommitFailedException> cleanFuture = t.submit();
+
+ this.domChain.close();
+
+ return cleanFuture;
}
@Override
@Override
public ClusterSingletonServiceRegistration registerClusterSingletonService(final ClusterSingletonService clusterSingletonService) {
- return this.provider.registerClusterSingletonService(clusterSingletonService);
+ return ClusterSingletonServiceRegistrationHelper.registerSingletonService(this.provider, clusterSingletonService, MAX_REGISTRATION_ATTEMPTS,
+ SLEEP_TIME);
}
@Override
@Override
public ClusterSingletonServiceRegistration registerClusterSingletonService(final ClusterSingletonService clusterSingletonService) {
- return this.provider.registerClusterSingletonService(clusterSingletonService);
+ return this.ribImpl.registerClusterSingletonService(clusterSingletonService);
}
}
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
-import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.primitives.Shorts;
import java.util.ArrayList;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.opendaylight.controller.md.sal.dom.api.DOMDataBroker;
-import org.opendaylight.controller.md.sal.dom.api.DOMDataBrokerExtension;
import org.opendaylight.controller.md.sal.dom.api.DOMDataTreeChangeService;
import org.opendaylight.controller.sal.core.api.model.SchemaService;
import org.opendaylight.mdsal.binding.dom.codec.api.BindingCodecTreeFactory;
import org.opendaylight.protocol.bgp.mode.impl.add.all.paths.AllPathSelection;
import org.opendaylight.protocol.bgp.openconfig.spi.BGPOpenConfigMappingService;
import org.opendaylight.protocol.bgp.parser.BgpTableTypeImpl;
-import org.opendaylight.protocol.bgp.rib.DefaultRibReference;
-import org.opendaylight.protocol.bgp.rib.RibReference;
import org.opendaylight.protocol.bgp.rib.impl.RIBImpl;
import org.opendaylight.protocol.bgp.rib.spi.RIBExtensionConsumerContext;
import org.opendaylight.protocol.bgp.rib.spi.RIBSupport;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev130919.BgpTableType;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.openconfig.extensions.rev160614.AfiSafi2;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.openconfig.extensions.rev160614.AfiSafi2Builder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.BgpRib;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.RibId;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.bgp.rib.Rib;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.bgp.rib.RibKey;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.TablesKey;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.BgpId;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.Ipv4AddressFamily;
import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev130919.UnicastSubsequentAddressFamily;
import org.opendaylight.yangtools.concepts.ListenerRegistration;
import org.opendaylight.yangtools.sal.binding.generator.impl.GeneratedClassLoadingStrategy;
-import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
import org.opendaylight.yangtools.yang.common.QName;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode;
import org.osgi.framework.ServiceRegistration;
@Mock
private ServiceRegistration serviceRegistration;
+ @Override
@Before
public void setUp() throws Exception {
super.setUp();
Mockito.doAnswer(new Answer<ClusterSingletonServiceRegistration>() {
@Override
public ClusterSingletonServiceRegistration answer(final InvocationOnMock invocationOnMock) throws Throwable {
- singletonService = (ClusterSingletonService) invocationOnMock.getArguments()[0];
- return singletonServiceRegistration;
+ RibImplTest.this.singletonService = (ClusterSingletonService) invocationOnMock.getArguments()[0];
+ return RibImplTest.this.singletonServiceRegistration;
}
}).when(this.clusterSingletonServiceProvider).registerClusterSingletonService(any(ClusterSingletonService.class));
verify(this.mappingService).toPathSelectionMode(anyList());
verify(this.mappingService).toTableTypes(anyList());
verify(this.extension).getClassLoadingStrategy();
- verify(this.domDataBroker).createTransactionChain(any(RIBImpl.class));
verify(this.domDataBroker).getSupportedExtensions();
verify(this.clusterSingletonServiceProvider).registerClusterSingletonService(any());
verify(this.schemaService).registerSchemaContextListener(any(RIBImpl.class));
<artifactId>org.osgi.core</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.opendaylight.mdsal</groupId>
+ <artifactId>mdsal-singleton-common-api</artifactId>
+ </dependency>
<!-- Test dependencies -->
<dependency>
--- /dev/null
+/*
+ * Copyright (c) 2016 Cisco Systems, Inc. and others. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.protocol.bgp.rib.spi.util;
+
+import com.google.common.util.concurrent.Uninterruptibles;
+import java.util.concurrent.TimeUnit;
+import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService;
+import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
+import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Utility class which provides helper functionality for ClusterSingletonService.
+ *
+ */
+public final class ClusterSingletonServiceRegistrationHelper {
+
+ private static final Logger LOG = LoggerFactory.getLogger(ClusterSingletonServiceRegistrationHelper.class);
+
+ private ClusterSingletonServiceRegistrationHelper() {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * This helper function wraps {@link ClusterSingletonServiceProvider#registerClusterSingletonService(ClusterSingletonService)} in order to
+ * execute repeated registration attempts while catching RuntimeException. If registration is not successful, RuntimeException is re-thrown.
+ * @param singletonProvider
+ * @param clusterSingletonService
+ * @param maxAttempts Upper bound for registration retries count.
+ * @param sleepTime Sleep time between registration retries in milliseconds.
+ * @return Registration
+ */
+ public static ClusterSingletonServiceRegistration registerSingletonService(final ClusterSingletonServiceProvider singletonProvider,
+ final ClusterSingletonService clusterSingletonService, final int maxAttempts, final int sleepTime) {
+ int attempts = maxAttempts;
+ while (true) {
+ try {
+ return singletonProvider.registerClusterSingletonService(clusterSingletonService);
+ } catch (final RuntimeException e) {
+ if (attempts == 0) {
+ LOG.error("Giving up after {} registration attempts for service {}.", maxAttempts, clusterSingletonService, e);
+ throw e;
+ }
+ attempts--;
+ LOG.warn("Failed to register {} service to ClusterSingletonServiceProvider. Try again in {} ms.", clusterSingletonService, sleepTime);
+ Uninterruptibles.sleepUninterruptibly(sleepTime, TimeUnit.MILLISECONDS);
+ }
+ }
+ }
+
+}
--- /dev/null
+/*
+ * Copyright (c) 2016 Cisco Systems, Inc. and others. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.protocol.bgp.rib.spi.util;
+
+import static org.opendaylight.protocol.bgp.rib.spi.util.ClusterSingletonServiceRegistrationHelper.registerSingletonService;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
+import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService;
+import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
+import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration;
+
+public class ClusterSingletonServiceRegistrationHelperTest {
+
+ @Mock
+ private ClusterSingletonService clusterSingletonService;
+ @Mock
+ private ClusterSingletonServiceProvider singletonProvider;
+ @Mock
+ private ClusterSingletonServiceRegistration serviceRegistration;
+
+ @Before
+ public void setUp() {
+ MockitoAnnotations.initMocks(this);
+ Mockito.doReturn(null).when(this.singletonProvider).registerClusterSingletonService(Mockito.any());
+ Mockito.when(this.singletonProvider.registerClusterSingletonService(Mockito.any()))
+ .thenThrow(new RuntimeException())
+ .thenReturn(this.serviceRegistration);
+ }
+
+ @Test(expected=UnsupportedOperationException.class)
+ public void testPrivateConstructor() throws Throwable {
+ final Constructor<ClusterSingletonServiceRegistrationHelper> c = ClusterSingletonServiceRegistrationHelper.class.getDeclaredConstructor();
+ c.setAccessible(true);
+ try {
+ c.newInstance();
+ } catch (final InvocationTargetException e) {
+ throw e.getCause();
+ }
+ }
+
+ @Test(expected=RuntimeException.class)
+ public void testRegisterSingletonServiceFailure() {
+ registerSingletonService(this.singletonProvider, this.clusterSingletonService, 0, 1);
+ }
+
+ @Test
+ public void testRegisterSingletonServiceSuccessfulRetry() {
+ final ClusterSingletonServiceRegistration registerSingletonService = registerSingletonService(this.singletonProvider, this.clusterSingletonService, 1, 1);
+ Assert.assertEquals(this.serviceRegistration, registerSingletonService);
+ //first reg. attempt failed, second succeeded
+ Mockito.verify(this.singletonProvider, Mockito.times(2)).registerClusterSingletonService(Mockito.any());
+ }
+
+}
import org.opendaylight.controller.md.sal.binding.api.DataTreeIdentifier;
import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService;
import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider;
import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration;
+import org.opendaylight.protocol.bgp.rib.spi.util.ClusterSingletonServiceRegistrationHelper;
import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology;
import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;
import org.opendaylight.yangtools.concepts.AbstractRegistration;
private static final Logger LOG = LoggerFactory.getLogger(BgpTopologyDeployerImpl.class);
+ private static final int MAX_REGISTRATION_ATTEMPTS = 10;
+ private static final int SLEEP_TIME = MAX_REGISTRATION_ATTEMPTS;
+
@GuardedBy("this")
private final Set<BgpTopologyProvider> topologyProviders = new HashSet<>();
private final DataBroker dataBroker;
this.dataBroker = Preconditions.checkNotNull(dataBroker);
this.singletonProvider = Preconditions.checkNotNull(singletonProvider);
this.registration =
- this.dataBroker.registerDataTreeChangeListener(new DataTreeIdentifier<Topology>(LogicalDatastoreType.CONFIGURATION, InstanceIdentifier.create(NetworkTopology.class).child(Topology.class)), this);
+ this.dataBroker.registerDataTreeChangeListener(new DataTreeIdentifier<>(LogicalDatastoreType.CONFIGURATION, InstanceIdentifier.create(NetworkTopology.class).child(Topology.class)), this);
LOG.info("BGP topology deployer started.");
}
final Dictionary<String, String> properties = new Hashtable<>();
properties.put("topology-id", topologyProviderService.getInstanceIdentifier().firstKeyOf(Topology.class).getTopologyId().getValue());
final ServiceRegistration<?> registerService = this.context.registerService(new String[] {TopologyReference.class.getName()}, topologyProviderService, properties);
- final ClusterSingletonServiceRegistration registerClusterSingletonService = this.singletonProvider.registerClusterSingletonService(topologyProviderService);
+ final ClusterSingletonServiceRegistration registerClusterSingletonService = registerSingletonService(topologyProviderService);
return new AbstractRegistration() {
@Override
protected void removeRegistration() {
filterTopologyBuilders(topology).forEach(provider -> provider.onTopologyBuilderRemoved(topology));
}
+ private ClusterSingletonServiceRegistration registerSingletonService(final ClusterSingletonService clusterSingletonService) {
+ return ClusterSingletonServiceRegistrationHelper.registerSingletonService(this.singletonProvider, clusterSingletonService, MAX_REGISTRATION_ATTEMPTS, SLEEP_TIME);
+ }
+
}
Code Review / bgpcep.git / commitdiff