From db71cd69ef6866fce737602bcda98798d2886d9c Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Fri, 20 Feb 2015 14:21:26 +0100 Subject: [PATCH] Fix race condition in get/get-config netconf rpcs for config subsystem 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 --- .../controller/config/api/ConfigRegistry.java | 19 ------ .../controller/config/api/LookupRegistry.java | 17 +++++ .../impl/ConfigTransactionControllerImpl.java | 19 ++++++ .../impl/ConfigTransactionLookupRegistry.java | 20 ++++++ .../impl/ServiceReferenceRegistryImpl.java | 10 +++ .../controller/config/util/BeanReader.java | 10 +++ .../config/util/ConfigRegistryClient.java | 4 +- .../config/util/ConfigRegistryJMXClient.java | 2 +- .../config/util/ConfigTransactionClient.java | 4 +- .../util/ConfigTransactionJMXClient.java | 41 ++++++++---- .../TestingConfigTransactionController.java | 11 ++++ .../mapping/config/InstanceConfig.java | 6 +- .../operations/Datastore.java | 2 +- .../operations/editconfig/EditConfig.java | 4 +- .../operations/get/Get.java | 37 +++++++---- .../operations/getconfig/GetConfig.java | 45 ++++++++----- .../RunningDatastoreQueryStrategy.java | 12 +++- .../osgi/NetconfOperationProvider.java | 2 +- .../transactions/TransactionProvider.java | 65 +++++++++++++++---- .../NetconfMappingTest.java | 2 +- 20 files changed, 243 insertions(+), 89 deletions(-) create mode 100644 opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/BeanReader.java diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ConfigRegistry.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ConfigRegistry.java index 5c9ed8767c..3d72114daf 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ConfigRegistry.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/ConfigRegistry.java @@ -66,23 +66,4 @@ public interface ConfigRegistry extends LookupRegistry, ServiceReferenceReadable Set getAvailableModuleNames(); - - /** - * Find all runtime beans - * - * @return objectNames - */ - Set lookupRuntimeBeans(); - - /** - * Find all runtime of specified module - * - * @param moduleName - * of bean - * @param instanceName - * of bean - * @return objectNames - */ - Set lookupRuntimeBeans(String moduleName, String instanceName); - } diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/LookupRegistry.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/LookupRegistry.java index b90fc9c034..5d615c2084 100644 --- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/LookupRegistry.java +++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/api/LookupRegistry.java @@ -71,4 +71,21 @@ public interface LookupRegistry { */ Set getAvailableModuleFactoryQNames(); + /** + * Find all runtime beans + * + * @return objectNames + */ + Set lookupRuntimeBeans(); + + /** + * Find all runtime of specified module + * + * @param moduleName + * of bean + * @param instanceName + * of bean + * @return objectNames + */ + Set lookupRuntimeBeans(String moduleName, String instanceName); } diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java index 37c2e2d777..186a7218ba 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionControllerImpl.java @@ -482,6 +482,25 @@ class ConfigTransactionControllerImpl implements public void checkConfigBeanExists(ObjectName objectName) throws InstanceNotFoundException { txLookupRegistry.checkConfigBeanExists(objectName); } + + + /** + * {@inheritDoc} + */ + @Override + public Set lookupRuntimeBeans() { + return txLookupRegistry.lookupRuntimeBeans(); + } + + /** + * {@inheritDoc} + */ + @Override + public Set lookupRuntimeBeans(String moduleName, + String instanceName) { + return txLookupRegistry.lookupRuntimeBeans(moduleName, instanceName); + } + // -- /** diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionLookupRegistry.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionLookupRegistry.java index f9a3801171..a0138b2d9d 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionLookupRegistry.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigTransactionLookupRegistry.java @@ -116,6 +116,26 @@ class ConfigTransactionLookupRegistry implements LookupRegistry, Closeable { return ModuleQNameUtil.getQNames(allCurrentFactories); } + /** + * {@inheritDoc} + */ + @Override + public Set lookupRuntimeBeans() { + return lookupRuntimeBeans("*", "*"); + } + + /** + * {@inheritDoc} + */ + @Override + public Set 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() { diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java index 27f0d5c1f2..dd6c2b9422 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java @@ -97,6 +97,16 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe throw new UnsupportedOperationException(); } + @Override + public Set lookupRuntimeBeans() { + throw new UnsupportedOperationException(); + } + + @Override + public Set lookupRuntimeBeans(final String moduleName, final String instanceName) { + throw new UnsupportedOperationException(); + } + @Override public String toString() { return "initial"; diff --git a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/BeanReader.java b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/BeanReader.java new file mode 100644 index 0000000000..de9baa6897 --- /dev/null +++ b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/BeanReader.java @@ -0,0 +1,10 @@ +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); +} diff --git a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigRegistryClient.java b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigRegistryClient.java index 99d46cb638..d384ae55c0 100644 --- a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigRegistryClient.java +++ b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigRegistryClient.java @@ -10,7 +10,7 @@ package org.opendaylight.controller.config.util; 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(); @@ -23,6 +23,4 @@ public interface ConfigRegistryClient extends ConfigRegistryMXBean { Object invokeMethod(ObjectName on, String name, Object[] params, String[] signature); - Object getAttributeCurrentValue(ObjectName on, String attributeName); - } diff --git a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigRegistryJMXClient.java b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigRegistryJMXClient.java index 099d010642..a39111afee 100644 --- a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigRegistryJMXClient.java +++ b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigRegistryJMXClient.java @@ -211,7 +211,7 @@ public class ConfigRegistryJMXClient implements ConfigRegistryClient { } catch (AttributeNotFoundException | InstanceNotFoundException | MBeanException | ReflectionException e) { throw new RuntimeException("Unable to get attribute " - + attributeName + " for " + on, e); + + attributeName + " for " + on + ". Available beans: " + lookupConfigBeans(), e); } } diff --git a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigTransactionClient.java b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigTransactionClient.java index 359035d51d..c7c072d39d 100644 --- a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigTransactionClient.java +++ b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigTransactionClient.java @@ -16,7 +16,7 @@ import org.opendaylight.controller.config.api.jmx.CommitStatus; import org.opendaylight.controller.config.api.jmx.ConfigTransactionControllerMXBean; public interface ConfigTransactionClient extends - ConfigTransactionControllerMXBean { + ConfigTransactionControllerMXBean, BeanReader { CommitStatus commit() throws ConflictingVersionException, ValidationException; @@ -47,7 +47,7 @@ public interface ConfigTransactionClient extends * @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); } diff --git a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigTransactionJMXClient.java b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigTransactionJMXClient.java index a0af19796e..26ca1391ad 100644 --- a/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigTransactionJMXClient.java +++ b/opendaylight/config/config-util/src/main/java/org/opendaylight/controller/config/util/ConfigTransactionJMXClient.java @@ -240,6 +240,26 @@ public class ConfigTransactionJMXClient implements ConfigTransactionClient { 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 { @@ -273,22 +293,17 @@ public class ConfigTransactionJMXClient implements ConfigTransactionClient { } @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 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 lookupRuntimeBeans() { + return configTransactionControllerMXBeanProxy.lookupRuntimeBeans(); } @Override - public Set getAvailableModuleFactoryQNames() { - return configTransactionControllerMXBeanProxy.getAvailableModuleFactoryQNames(); + public Set lookupRuntimeBeans(final String moduleName, final String instanceName) { + return configTransactionControllerMXBeanProxy.lookupRuntimeBeans(moduleName, instanceName); } } diff --git a/opendaylight/config/config-util/src/test/java/org/opendaylight/controller/config/util/TestingConfigTransactionController.java b/opendaylight/config/config-util/src/test/java/org/opendaylight/controller/config/util/TestingConfigTransactionController.java index e1138addc7..e69019405d 100644 --- a/opendaylight/config/config-util/src/test/java/org/opendaylight/controller/config/util/TestingConfigTransactionController.java +++ b/opendaylight/config/config-util/src/test/java/org/opendaylight/controller/config/util/TestingConfigTransactionController.java @@ -8,6 +8,7 @@ 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; @@ -184,6 +185,16 @@ public class TestingConfigTransactionController implements return Sets.newHashSet("availableModuleFactoryQNames"); } + @Override + public Set lookupRuntimeBeans() { + return Collections.emptySet(); + } + + @Override + public Set lookupRuntimeBeans(final String moduleName, final String instanceName) { + return Collections.emptySet(); + } + @Override public ObjectName getServiceReference(String serviceInterfaceQName, String refName) throws InstanceNotFoundException { return conf3; diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/mapping/config/InstanceConfig.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/mapping/config/InstanceConfig.java index 4ae4de18f3..ba7b2f20e4 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/mapping/config/InstanceConfig.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/mapping/config/InstanceConfig.java @@ -17,7 +17,7 @@ import java.util.Map; 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; @@ -46,9 +46,9 @@ public final class InstanceConfig { private final Map yangToAttrConfig; private final String nullableDummyContainerName; private final Map jmxToAttrConfig; - private final ConfigRegistryClient configRegistryClient; + private final BeanReader configRegistryClient; - public InstanceConfig(ConfigRegistryClient configRegistryClient, Map yangNamesToAttributes, + public InstanceConfig(BeanReader configRegistryClient, Map yangNamesToAttributes, String nullableDummyContainerName) { this.yangToAttrConfig = yangNamesToAttributes; diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/Datastore.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/Datastore.java index d736595719..9c55953bbc 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/Datastore.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/Datastore.java @@ -26,7 +26,7 @@ public enum Datastore { TransactionProvider transactionProvider) { switch (source) { case running: - return new RunningDatastoreQueryStrategy(); + return new RunningDatastoreQueryStrategy(transactionProvider); case candidate: return new CandidateDatastoreQueryStrategy(transactionProvider); default: diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/editconfig/EditConfig.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/editconfig/EditConfig.java index ca6a8c46b9..bc84734190 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/editconfig/EditConfig.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/editconfig/EditConfig.java @@ -20,6 +20,7 @@ import java.util.Set; 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; @@ -262,7 +263,7 @@ public class EditConfig extends AbstractConfigNetconfOperation { public static Map> transformMbeToModuleConfigs - (final ConfigRegistryClient configRegistryClient, Map> mBeanEntries) { Map> namespaceToModuleNameToModuleConfig = Maps.newHashMap(); @@ -295,7 +296,6 @@ public class EditConfig extends AbstractConfigNetconfOperation { @Override protected Element handleWithNoSubsequentOperations(Document document, XmlElement xml) throws NetconfDocumentedException { - EditConfigXmlParser.EditConfigExecution editConfigExecution; Config cfg = getConfigMapping(getConfigRegistryClient(), yangStoreSnapshot); editConfigExecution = editConfigXmlParser.fromXml(xml, cfg); diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/get/Get.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/get/Get.java index 27d53cdc32..fe7f2773cd 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/get/Get.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/get/Get.java @@ -14,6 +14,7 @@ import java.util.Map; 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; @@ -27,6 +28,7 @@ import org.opendaylight.controller.netconf.confignetconfconnector.operations.Abs 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; @@ -38,12 +40,14 @@ import org.w3c.dom.Element; 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; } @@ -115,23 +119,30 @@ public class Get extends AbstractConfigNetconfOperation { protected Element handleWithNoSubsequentOperations(Document document, XmlElement xml) throws NetconfDocumentedException { checkXml(xml); - final Set runtimeBeans = getConfigRegistryClient().lookupRuntimeBeans(); + final ObjectName testTransaction = transactionProvider.getOrCreateReadTransaction(); + final ConfigTransactionClient registryClient = getConfigRegistryClient().getConfigTransactionClient(testTransaction); - //Transaction provider required only for candidate datastore - final Set 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 runtimeBeans = getConfigRegistryClient().lookupRuntimeBeans(); - final Map> moduleRuntimes = createModuleRuntimes(getConfigRegistryClient(), - yangStoreSnapshot.getModuleMXBeanEntryMap()); - final Map> moduleConfigs = EditConfig.transformMbeToModuleConfigs( - getConfigRegistryClient(), yangStoreSnapshot.getModuleMXBeanEntryMap()); + final Set configBeans = Datastore.getInstanceQueryStrategy(Datastore.running, transactionProvider) + .queryInstances(getConfigRegistryClient()); - final Runtime runtime = new Runtime(moduleRuntimes, moduleConfigs); + final Map> moduleRuntimes = createModuleRuntimes(getConfigRegistryClient(), + yangStoreSnapshot.getModuleMXBeanEntryMap()); + final Map> 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(); + } } } diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/getconfig/GetConfig.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/getconfig/GetConfig.java index 350ace5eb1..2c4bde0ee8 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/getconfig/GetConfig.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/getconfig/GetConfig.java @@ -72,23 +72,36 @@ public class GetConfig extends AbstractConfigNetconfOperation { private Element getResponseInternal(final Document document, final ConfigRegistryClient configRegistryClient, final Datastore source) { - Element dataElement = XmlUtil.createElement(document, XmlNetconfConstants.DATA_KEY, Optional.absent()); - final Set 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.absent()); + final Set 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 diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/getconfig/RunningDatastoreQueryStrategy.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/getconfig/RunningDatastoreQueryStrategy.java index ae9cb2eb37..74b5f60a10 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/getconfig/RunningDatastoreQueryStrategy.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/operations/getconfig/RunningDatastoreQueryStrategy.java @@ -11,12 +11,22 @@ package org.opendaylight.controller.netconf.confignetconfconnector.operations.ge 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 queryInstances(ConfigRegistryClient configRegistryClient) { - return configRegistryClient.lookupConfigBeans(); + ObjectName on = transactionProvider.getOrCreateReadTransaction(); + ConfigTransactionClient proxy = configRegistryClient.getConfigTransactionClient(on); + return proxy.lookupConfigBeans(); } } diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationProvider.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationProvider.java index 612bd85998..e3fdc056d9 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationProvider.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationProvider.java @@ -52,7 +52,7 @@ final class NetconfOperationProvider { 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)); diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java index b2ee63a987..7655cb300d 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/transactions/TransactionProvider.java @@ -31,7 +31,8 @@ public class TransactionProvider implements AutoCloseable { private final ConfigRegistryClient configRegistryClient; private final String netconfSessionIdForReporting; - private ObjectName transaction; + private ObjectName candidateTx; + private ObjectName readTx; private final List allOpenedTransactions = new ArrayList<>(); private static final String NO_TRANSACTION_FOUND_FOR_SESSION = "No transaction found for session "; @@ -56,18 +57,34 @@ public class TransactionProvider implements AutoCloseable { public synchronized Optional 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 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) { @@ -80,9 +97,20 @@ public class TransactionProvider implements AutoCloseable { 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 ta = getReadTransaction(); + + if (ta.isPresent()) { + return ta.get(); + } + readTx = configRegistryClient.beginConfig(); + allOpenedTransactions.add(readTx); + return readTx; } /** @@ -109,8 +137,8 @@ public class TransactionProvider implements AutoCloseable { 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 @@ -131,8 +159,19 @@ public class TransactionProvider implements AutoCloseable { 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 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) { diff --git a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java index f1fc27725b..136af5b3c6 100644 --- a/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java +++ b/opendaylight/netconf/config-netconf-connector/src/test/java/org/opendaylight/controller/netconf/confignetconfconnector/NetconfMappingTest.java @@ -715,7 +715,7 @@ public class NetconfMappingTest extends AbstractConfigTest { } 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"); } -- 2.36.6