The read and transformation of modules from config subsystem is performed in a couple of steps. Each step queried the config subsystem on its own so it was possible to cause inconsistencies among these steps if get/get-config was executed while another transaction was being committed. This race condition, if hit, caused the rpcs to fail with an exception of a missing MBean.
After recent introduction of "optional reconnect after capability changed" feature into sal-netconf-connector, this race condition caused get rpc invoked by the connector to fail.
Scenario:
odl-netconf-connector-all is installed,
after the loopback is connected any other feature with configuration is installed e.g. odl-restconf-all,
loopback connector reconnects and as it comes back executes get rpc (to query ietf-netconf-monitoring)
get rpc in config-netconf-connector fails to transform the modules into xml due to mentioned race condition as the initial config file for restconf has been pushed
Now the get/get-config read the data from a dedicated transaction started just for the read.
Note: get rpc reads the runtime beans that are part of no transaction, so there is still possibility for this race condition regarding runtime beans.
Change-Id: I0822bc48745f9f680b116095693052dff752dee3
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
Set<String> getAvailableModuleNames();
-
- /**
- * Find all runtime beans
- *
- * @return objectNames
- */
- Set<ObjectName> lookupRuntimeBeans();
-
- /**
- * Find all runtime of specified module
- *
- * @param moduleName
- * of bean
- * @param instanceName
- * of bean
- * @return objectNames
- */
- Set<ObjectName> lookupRuntimeBeans(String moduleName, String instanceName);
-
}
*/
Set<String> getAvailableModuleFactoryQNames();
+ /**
+ * Find all runtime beans
+ *
+ * @return objectNames
+ */
+ Set<ObjectName> lookupRuntimeBeans();
+
+ /**
+ * Find all runtime of specified module
+ *
+ * @param moduleName
+ * of bean
+ * @param instanceName
+ * of bean
+ * @return objectNames
+ */
+ Set<ObjectName> lookupRuntimeBeans(String moduleName, String instanceName);
}
public void checkConfigBeanExists(ObjectName objectName) throws InstanceNotFoundException {
txLookupRegistry.checkConfigBeanExists(objectName);
}
+
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans() {
+ return txLookupRegistry.lookupRuntimeBeans();
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans(String moduleName,
+ String instanceName) {
+ return txLookupRegistry.lookupRuntimeBeans(moduleName, instanceName);
+ }
+
// --
/**
return ModuleQNameUtil.getQNames(allCurrentFactories);
}
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans() {
+ return lookupRuntimeBeans("*", "*");
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans(String moduleName,
+ String instanceName) {
+ String finalModuleName = moduleName == null ? "*" : moduleName;
+ String finalInstanceName = instanceName == null ? "*" : instanceName;
+ ObjectName namePattern = ObjectNameUtil.createRuntimeBeanPattern(
+ finalModuleName, finalInstanceName);
+ return transactionJMXRegistrator.queryNames(namePattern, null);
+ }
@Override
public String toString() {
throw new UnsupportedOperationException();
}
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans(final String moduleName, final String instanceName) {
+ throw new UnsupportedOperationException();
+ }
+
@Override
public String toString() {
return "initial";
--- /dev/null
+package org.opendaylight.controller.config.util;
+
+import javax.management.ObjectName;
+
+/**
+ * Created by mmarsale on 20.2.2015.
+ */
+public interface BeanReader {
+ Object getAttributeCurrentValue(ObjectName on, String attributeName);
+}
import javax.management.ObjectName;
import org.opendaylight.controller.config.api.jmx.ConfigRegistryMXBean;
-public interface ConfigRegistryClient extends ConfigRegistryMXBean {
+public interface ConfigRegistryClient extends ConfigRegistryMXBean, BeanReader {
ConfigTransactionClient createTransaction();
Object invokeMethod(ObjectName on, String name, Object[] params,
String[] signature);
- Object getAttributeCurrentValue(ObjectName on, String attributeName);
-
}
} catch (AttributeNotFoundException | InstanceNotFoundException
| MBeanException | ReflectionException e) {
throw new RuntimeException("Unable to get attribute "
- + attributeName + " for " + on, e);
+ + attributeName + " for " + on + ". Available beans: " + lookupConfigBeans(), e);
}
}
import org.opendaylight.controller.config.api.jmx.ConfigTransactionControllerMXBean;
public interface ConfigTransactionClient extends
- ConfigTransactionControllerMXBean {
+ ConfigTransactionControllerMXBean, BeanReader {
CommitStatus commit() throws ConflictingVersionException,
ValidationException;
* @param on - ObjectName of the Object from which the attribute should be read
* @param jmxName - name of the attribute to be read
*
- * @return Attribute of Object on with attribute name jmxName
+ * @return Object of Object on with attribute name jmxName
*/
Attribute getAttribute(ObjectName on, String jmxName);
}
configTransactionControllerMXBeanProxy.checkServiceReferenceExists(objectName);
}
+ @Override
+ public Attribute getAttribute(ObjectName on, String attrName) {
+ if (ObjectNameUtil.getTransactionName(on) == null) {
+ throw new IllegalArgumentException("Not in transaction instance "
+ + on + ", no transaction name present");
+ }
+
+ try {
+ return new Attribute(attrName, configMBeanServer.getAttribute(on,attrName));
+ } catch (JMException e) {
+ throw new IllegalStateException("Unable to get attribute "
+ + attrName + " for " + on, e);
+ }
+ }
+
+ @Override
+ public Object getAttributeCurrentValue(ObjectName on, String attrName) {
+ return getAttribute(on, attrName).getValue();
+ }
+
@Override
public void validateBean(ObjectName configBeanON)
throws ValidationException {
}
@Override
- public Attribute getAttribute(ObjectName on, String attrName) {
- if (ObjectNameUtil.getTransactionName(on) == null) {
- throw new IllegalArgumentException("Not in transaction instance "
- + on + ", no transaction name present");
- }
+ public Set<String> getAvailableModuleFactoryQNames() {
+ return configTransactionControllerMXBeanProxy.getAvailableModuleFactoryQNames();
+ }
- try {
- return new Attribute(attrName, configMBeanServer.getAttribute(on,attrName));
- } catch (JMException e) {
- throw new IllegalStateException("Unable to get attribute "
- + attrName + " for " + on, e);
- }
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans() {
+ return configTransactionControllerMXBeanProxy.lookupRuntimeBeans();
}
@Override
- public Set<String> getAvailableModuleFactoryQNames() {
- return configTransactionControllerMXBeanProxy.getAvailableModuleFactoryQNames();
+ public Set<ObjectName> lookupRuntimeBeans(final String moduleName, final String instanceName) {
+ return configTransactionControllerMXBeanProxy.lookupRuntimeBeans(moduleName, instanceName);
}
}
package org.opendaylight.controller.config.util;
import com.google.common.collect.Sets;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
return Sets.newHashSet("availableModuleFactoryQNames");
}
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans() {
+ return Collections.emptySet();
+ }
+
+ @Override
+ public Set<ObjectName> lookupRuntimeBeans(final String moduleName, final String instanceName) {
+ return Collections.emptySet();
+ }
+
@Override
public ObjectName getServiceReference(String serviceInterfaceQName, String refName) throws InstanceNotFoundException {
return conf3;
import java.util.Map.Entry;
import javax.management.ObjectName;
import javax.management.openmbean.OpenType;
-import org.opendaylight.controller.config.util.ConfigRegistryClient;
+import org.opendaylight.controller.config.util.BeanReader;
import org.opendaylight.controller.config.yangjmxgenerator.RuntimeBeanEntry;
import org.opendaylight.controller.config.yangjmxgenerator.attribute.AttributeIfc;
import org.opendaylight.controller.netconf.api.NetconfDocumentedException;
private final Map<String, AttributeIfc> yangToAttrConfig;
private final String nullableDummyContainerName;
private final Map<String, AttributeIfc> jmxToAttrConfig;
- private final ConfigRegistryClient configRegistryClient;
+ private final BeanReader configRegistryClient;
- public InstanceConfig(ConfigRegistryClient configRegistryClient, Map<String, AttributeIfc> yangNamesToAttributes,
+ public InstanceConfig(BeanReader configRegistryClient, Map<String, AttributeIfc> yangNamesToAttributes,
String nullableDummyContainerName) {
this.yangToAttrConfig = yangNamesToAttributes;
TransactionProvider transactionProvider) {
switch (source) {
case running:
- return new RunningDatastoreQueryStrategy();
+ return new RunningDatastoreQueryStrategy(transactionProvider);
case candidate:
return new CandidateDatastoreQueryStrategy(transactionProvider);
default:
import javax.management.InstanceNotFoundException;
import javax.management.ObjectName;
import org.opendaylight.controller.config.api.ValidationException;
+import org.opendaylight.controller.config.util.BeanReader;
import org.opendaylight.controller.config.util.ConfigRegistryClient;
import org.opendaylight.controller.config.util.ConfigTransactionClient;
import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry;
public static Map<String/* Namespace from yang file */,
Map<String /* Name of module entry from yang file */, ModuleConfig>> transformMbeToModuleConfigs
- (final ConfigRegistryClient configRegistryClient, Map<String/* Namespace from yang file */,
+ (final BeanReader configRegistryClient, Map<String/* Namespace from yang file */,
Map<String /* Name of module entry from yang file */, ModuleMXBeanEntry>> mBeanEntries) {
Map<String, Map<String, ModuleConfig>> namespaceToModuleNameToModuleConfig = Maps.newHashMap();
@Override
protected Element handleWithNoSubsequentOperations(Document document, XmlElement xml) throws NetconfDocumentedException {
-
EditConfigXmlParser.EditConfigExecution editConfigExecution;
Config cfg = getConfigMapping(getConfigRegistryClient(), yangStoreSnapshot);
editConfigExecution = editConfigXmlParser.fromXml(xml, cfg);
import java.util.Set;
import javax.management.ObjectName;
import org.opendaylight.controller.config.util.ConfigRegistryClient;
+import org.opendaylight.controller.config.util.ConfigTransactionClient;
import org.opendaylight.controller.config.yangjmxgenerator.ModuleMXBeanEntry;
import org.opendaylight.controller.config.yangjmxgenerator.RuntimeBeanEntry;
import org.opendaylight.controller.netconf.api.NetconfDocumentedException;
import org.opendaylight.controller.netconf.confignetconfconnector.operations.Datastore;
import org.opendaylight.controller.netconf.confignetconfconnector.operations.editconfig.EditConfig;
import org.opendaylight.controller.netconf.confignetconfconnector.osgi.YangStoreContext;
+import org.opendaylight.controller.netconf.confignetconfconnector.transactions.TransactionProvider;
import org.opendaylight.controller.netconf.util.exception.MissingNameSpaceException;
import org.opendaylight.controller.netconf.util.exception.UnexpectedElementException;
import org.opendaylight.controller.netconf.util.exception.UnexpectedNamespaceException;
public class Get extends AbstractConfigNetconfOperation {
+ private final TransactionProvider transactionProvider;
private final YangStoreContext yangStoreSnapshot;
private static final Logger LOG = LoggerFactory.getLogger(Get.class);
- public Get(YangStoreContext yangStoreSnapshot, ConfigRegistryClient configRegistryClient,
+ public Get(final TransactionProvider transactionProvider, YangStoreContext yangStoreSnapshot, ConfigRegistryClient configRegistryClient,
String netconfSessionIdForReporting) {
super(configRegistryClient, netconfSessionIdForReporting);
+ this.transactionProvider = transactionProvider;
this.yangStoreSnapshot = yangStoreSnapshot;
}
protected Element handleWithNoSubsequentOperations(Document document, XmlElement xml) throws NetconfDocumentedException {
checkXml(xml);
- final Set<ObjectName> runtimeBeans = getConfigRegistryClient().lookupRuntimeBeans();
+ final ObjectName testTransaction = transactionProvider.getOrCreateReadTransaction();
+ final ConfigTransactionClient registryClient = getConfigRegistryClient().getConfigTransactionClient(testTransaction);
- //Transaction provider required only for candidate datastore
- final Set<ObjectName> configBeans = Datastore.getInstanceQueryStrategy(Datastore.running, null)
- .queryInstances(getConfigRegistryClient());
+ try {
+ // Runtime beans are not parts of transactions and have to be queried against the central registry
+ final Set<ObjectName> runtimeBeans = getConfigRegistryClient().lookupRuntimeBeans();
- final Map<String, Map<String, ModuleRuntime>> moduleRuntimes = createModuleRuntimes(getConfigRegistryClient(),
- yangStoreSnapshot.getModuleMXBeanEntryMap());
- final Map<String, Map<String, ModuleConfig>> moduleConfigs = EditConfig.transformMbeToModuleConfigs(
- getConfigRegistryClient(), yangStoreSnapshot.getModuleMXBeanEntryMap());
+ final Set<ObjectName> configBeans = Datastore.getInstanceQueryStrategy(Datastore.running, transactionProvider)
+ .queryInstances(getConfigRegistryClient());
- final Runtime runtime = new Runtime(moduleRuntimes, moduleConfigs);
+ final Map<String, Map<String, ModuleRuntime>> moduleRuntimes = createModuleRuntimes(getConfigRegistryClient(),
+ yangStoreSnapshot.getModuleMXBeanEntryMap());
+ final Map<String, Map<String, ModuleConfig>> moduleConfigs = EditConfig.transformMbeToModuleConfigs(
+ registryClient, yangStoreSnapshot.getModuleMXBeanEntryMap());
- final Element element = runtime.toXml(runtimeBeans, configBeans, document);
+ final Runtime runtime = new Runtime(moduleRuntimes, moduleConfigs);
- LOG.trace("{} operation successful", XmlNetconfConstants.GET);
+ final Element element = runtime.toXml(runtimeBeans, configBeans, document);
- return element;
+ LOG.trace("{} operation successful", XmlNetconfConstants.GET);
+
+ return element;
+ } finally {
+ transactionProvider.closeReadTransaction();
+ }
}
}
private Element getResponseInternal(final Document document, final ConfigRegistryClient configRegistryClient,
final Datastore source) {
- Element dataElement = XmlUtil.createElement(document, XmlNetconfConstants.DATA_KEY, Optional.<String>absent());
- final Set<ObjectName> instances = Datastore.getInstanceQueryStrategy(source, this.transactionProvider)
- .queryInstances(configRegistryClient);
- final Config configMapping = new Config(EditConfig.transformMbeToModuleConfigs(configRegistryClient,
- yangStoreSnapshot.getModuleMXBeanEntryMap()));
-
-
- ObjectName on = transactionProvider.getOrCreateTransaction();
- ConfigTransactionClient ta = configRegistryClient.getConfigTransactionClient(on);
-
- ServiceRegistryWrapper serviceTracker = new ServiceRegistryWrapper(ta);
- dataElement = configMapping.toXml(instances, this.maybeNamespace, document, dataElement, serviceTracker);
-
- LOG.trace("{} operation successful", GET_CONFIG);
-
- return dataElement;
+ final ConfigTransactionClient registryClient;
+ // Read current state from a transaction, if running is source, then start new transaction just for reading
+ // in case of candidate, get current transaction representing candidate
+ if(source == Datastore.running) {
+ final ObjectName readTx = transactionProvider.getOrCreateReadTransaction();
+ registryClient = getConfigRegistryClient().getConfigTransactionClient(readTx);
+ } else {
+ registryClient = getConfigRegistryClient().getConfigTransactionClient(transactionProvider.getOrCreateTransaction());
+ }
+
+ try {
+ Element dataElement = XmlUtil.createElement(document, XmlNetconfConstants.DATA_KEY, Optional.<String>absent());
+ final Set<ObjectName> instances = Datastore.getInstanceQueryStrategy(source, this.transactionProvider)
+ .queryInstances(configRegistryClient);
+
+ final Config configMapping = new Config(EditConfig.transformMbeToModuleConfigs(registryClient,
+ yangStoreSnapshot.getModuleMXBeanEntryMap()));
+
+ ServiceRegistryWrapper serviceTracker = new ServiceRegistryWrapper(registryClient);
+ dataElement = configMapping.toXml(instances, this.maybeNamespace, document, dataElement, serviceTracker);
+
+ LOG.trace("{} operation successful", GET_CONFIG);
+
+ return dataElement;
+ } finally {
+ if(source == Datastore.running) {
+ transactionProvider.closeReadTransaction();
+ }
+ }
}
@Override
import java.util.Set;
import javax.management.ObjectName;
import org.opendaylight.controller.config.util.ConfigRegistryClient;
+import org.opendaylight.controller.config.util.ConfigTransactionClient;
+import org.opendaylight.controller.netconf.confignetconfconnector.transactions.TransactionProvider;
public class RunningDatastoreQueryStrategy implements DatastoreQueryStrategy {
+ private final TransactionProvider transactionProvider;
+
+ public RunningDatastoreQueryStrategy(TransactionProvider transactionProvider) {
+ this.transactionProvider = transactionProvider;
+ }
+
@Override
public Set<ObjectName> queryInstances(ConfigRegistryClient configRegistryClient) {
- return configRegistryClient.lookupConfigBeans();
+ ObjectName on = transactionProvider.getOrCreateReadTransaction();
+ ConfigTransactionClient proxy = configRegistryClient.getConfigTransactionClient(on);
+ return proxy.lookupConfigBeans();
}
}
ops.add(new Commit(transactionProvider, configRegistryClient, netconfSessionIdForReporting));
ops.add(new Lock(netconfSessionIdForReporting));
ops.add(new UnLock(netconfSessionIdForReporting));
- ops.add(new Get(yangStoreSnapshot, configRegistryClient, netconfSessionIdForReporting));
+ ops.add(new Get(transactionProvider, yangStoreSnapshot, configRegistryClient, netconfSessionIdForReporting));
ops.add(new DiscardChanges(transactionProvider, configRegistryClient, netconfSessionIdForReporting));
ops.add(new Validate(transactionProvider, configRegistryClient, netconfSessionIdForReporting));
ops.add(new RuntimeRpc(yangStoreSnapshot, configRegistryClient, netconfSessionIdForReporting));
private final ConfigRegistryClient configRegistryClient;
private final String netconfSessionIdForReporting;
- private ObjectName transaction;
+ private ObjectName candidateTx;
+ private ObjectName readTx;
private final List<ObjectName> allOpenedTransactions = new ArrayList<>();
private static final String NO_TRANSACTION_FOUND_FOR_SESSION = "No transaction found for session ";
public synchronized Optional<ObjectName> getTransaction() {
- if (transaction == null){
+ if (candidateTx == null){
return Optional.absent();
}
// Transaction was already closed somehow
- if (!isStillOpenTransaction(transaction)) {
- LOG.warn("Fixing illegal state: transaction {} was closed in {}", transaction,
+ if (!isStillOpenTransaction(candidateTx)) {
+ LOG.warn("Fixing illegal state: transaction {} was closed in {}", candidateTx,
netconfSessionIdForReporting);
- transaction = null;
+ candidateTx = null;
return Optional.absent();
}
- return Optional.of(transaction);
+ return Optional.of(candidateTx);
+ }
+
+ public synchronized Optional<ObjectName> getReadTransaction() {
+
+ if (readTx == null){
+ return Optional.absent();
+ }
+
+ // Transaction was already closed somehow
+ if (!isStillOpenTransaction(readTx)) {
+ LOG.warn("Fixing illegal state: transaction {} was closed in {}", readTx,
+ netconfSessionIdForReporting);
+ readTx = null;
+ return Optional.absent();
+ }
+ return Optional.of(readTx);
}
private boolean isStillOpenTransaction(ObjectName transaction) {
if (ta.isPresent()) {
return ta.get();
}
- transaction = configRegistryClient.beginConfig();
- allOpenedTransactions.add(transaction);
- return transaction;
+ candidateTx = configRegistryClient.beginConfig();
+ allOpenedTransactions.add(candidateTx);
+ return candidateTx;
+ }
+
+ public synchronized ObjectName getOrCreateReadTransaction() {
+ Optional<ObjectName> ta = getReadTransaction();
+
+ if (ta.isPresent()) {
+ return ta.get();
+ }
+ readTx = configRegistryClient.beginConfig();
+ allOpenedTransactions.add(readTx);
+ return readTx;
}
/**
try {
CommitStatus status = configRegistryClient.commitConfig(taON);
// clean up
- allOpenedTransactions.remove(transaction);
- transaction = null;
+ allOpenedTransactions.remove(candidateTx);
+ candidateTx = null;
return status;
} catch (ValidationException validationException) {
// no clean up: user can reconfigure and recover this transaction
ConfigTransactionClient transactionClient = configRegistryClient.getConfigTransactionClient(taON.get());
transactionClient.abortConfig();
- allOpenedTransactions.remove(transaction);
- transaction = null;
+ allOpenedTransactions.remove(candidateTx);
+ candidateTx = null;
+ }
+
+ public synchronized void closeReadTransaction() {
+ LOG.debug("Closing read transaction");
+ Optional<ObjectName> taON = getReadTransaction();
+ Preconditions.checkState(taON.isPresent(), NO_TRANSACTION_FOUND_FOR_SESSION + netconfSessionIdForReporting);
+
+ ConfigTransactionClient transactionClient = configRegistryClient.getConfigTransactionClient(taON.get());
+ transactionClient.abortConfig();
+ allOpenedTransactions.remove(readTx);
+ readTx = null;
}
public synchronized void abortTestTransaction(ObjectName testTx) {
}
private Document get() throws NetconfDocumentedException, ParserConfigurationException, SAXException, IOException {
- Get getOp = new Get(yangStoreSnapshot, configRegistryClient, NETCONF_SESSION_ID);
+ Get getOp = new Get(transactionProvider, yangStoreSnapshot, configRegistryClient, NETCONF_SESSION_ID);
return executeOp(getOp, "netconfMessages/get.xml");
}