camel-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Grzegorz Grzybek (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CAMEL-9313) CamelBlueprintTestSupport - timing problems with property placeholder
Date Thu, 19 Nov 2015 13:53:11 GMT

    [ https://issues.apache.org/jira/browse/CAMEL-9313?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15013567#comment-15013567
] 

Grzegorz Grzybek edited comment on CAMEL-9313 at 11/19/15 1:53 PM:
-------------------------------------------------------------------

Thanks for the analysis [~asiepert].

You're right - this has changed after 2.15.2. And it's true that if you put a breakpoint (to
hold "main" thread) in {{createBundleContext()}}, it fails in 2.15.2 too.

The reason is [this line|https://github.com/apache/camel/blob/camel-2.15.3/components/camel-test-blueprint/src/main/java/org/apache/camel/test/blueprint/CamelBlueprintTestSupport.java#L143].

The changes I've made are result of very unpredictable results of camel-test-blueprint tests
in CI.

To describe the problem, be aware that there are 3 groups of threads involved:
# {{main}} thread, where test is run - this is started by JUnit, maven-surefire-plugin, etc.
# threads related to aries blueprint
# threads related to felix configadmin (sometimes even started with {{new Thread(...).start()}}.

My ultimate goal was to achieve (at least) 100% reliability of the test - to eliminate all
race conditions. And the only way to do it is to do some kind of synchronization - using {{java.util.concurrent}}
primitives and listeners to BP / configadmin events.

So, when you add {{trace="{{tracing} }"}} to {{<camelContext>}}, then in 2.15.2 there's
great chance (unless you put a breakpoint to hold {{main}} thread), that BP container will
get initialized *after* [this|https://github.com/apache/camel/blob/camel-2.15.3/components/camel-test-blueprint/src/main/java/org/apache/camel/test/blueprint/CamelBlueprintTestSupport.java#L145-L150]:
{code:java}
if (file != null) {
  if (!new File(file[0]).exists()) {
    throw new IllegalArgumentException("The provided file \"" + file[0] + "\" from loadConfigAdminConfigurationFile
doesn't exist");
  }
  CamelBlueprintHelper.setPersistentFileForConfigAdmin(answer, file[1], file[0], props, symbolicName,
bpEvents, expectReload);
}
{code}
This leads to invocation of {{org.osgi.service.cm.Configuration#update(java.util.Dictionary<java.lang.String,
?>)}} and when (and there's high probability of this in 2.15.2) BP container isn't yet
started, it'll have those properties already available [when {{<cm:property-placeholder>}}
is initialized|https://github.com/apache/aries/blob/35c92e2c941f82db9c8815467e9173a5d710ab08/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedProperties.java#L140-L144]:
{code:java}
public void init() throws Exception {
    ...
    synchronized (lock) {
        managedObjectManager.register(this, props);
        Configuration config = CmUtils.getConfiguration(configAdmin, persistentId);
        if (config != null) {
            properties = config.getProperties();
        }
        updated(properties);
    }
}
{code}

Plus remember - there are two kinds of property placeholders: from Camel and from Aries-Blueprint.
Camel version delegates (in OSGi) to Aries version.

In order to have working {{trace="{{tracing} }"}} in 2.15.3+, you have to override simple
method: {{org.apache.camel.test.junit4.CamelTestSupport#useOverridePropertiesWithPropertiesComponent}}.

Add:
{code:java}
    @Override
    protected Properties useOverridePropertiesWithPropertiesComponent() {
        Properties props = new Properties();
        props.setProperty("tracing", "true");
        return props;
    }
{code}
to {{org.apache.camel.test.blueprint.ConfigAdminNoReloadLoadConfigurationFileTest}} and {{trace="{{tracing}
}"}} to {{org/apache/camel/test/blueprint/configadmin-no-reload-loadfile.xml}} and everything
will work, because PropertyComponent will have needed property when CamelContext is created.

In other words - my change in 2.15.3 related to predictability of camel-test-blueprint tests
can be summarized like this:
* we don't test (because of explicit synchronization) the scenarios, when configadmin configurations
are updated before blueprint container starts
* we test scenarios, where configadmin configuration updates should (when there's {{<cm:property-placeholder
... update-strategy="reload">}}) lead to reinitialization of blueprint container - and
to make it predictable, we have to synchrozize BP and configadmin events

It worked in 2.15.2 as a kind of side effect - just like the configadmin configuration already
existed when blueprint container was created for the first time. but it's not different then
specifying it explicitly with:
{code:xml}
  <cm:property-placeholder persistent-id="stuff" update-strategy="none">
    <cm:default-properties>
      <cm:property name="x" value="y"/>
    </cm:default-properties>
  </cm:property-placeholder>
{code}

And yet another words: {{CamelBlueprintTestSupport}} calls {{org.osgi.service.cm.Configuration#update(java.util.Dictionary<java.lang.String,?>)}}
*only* to change configadmin configs, not to init them.

I hope this resolves your issue.


was (Author: gzres):
Thanks for the analysis [~asiepert].

You're right - this has changed after 2.15.2. And it's true that if you put a breakpoint (to
hold "main" thread) in {{createBundleContext()}}, it fails in 2.15.2 too.

The reason is [this line|https://github.com/apache/camel/blob/camel-2.15.3/components/camel-test-blueprint/src/main/java/org/apache/camel/test/blueprint/CamelBlueprintTestSupport.java#L143].

The changes I've made are result of very unpredictable results of camel-test-blueprint tests
in CI.

To describe the problem, be aware that there are 3 groups of threads involved:
# {{main}} thread, where test is run - this is started by JUnit, maven-surefire-plugin, etc.
# threads related to aries blueprint
# threads related to felix configadmin (sometimes even started with {{new Thread(...).start()}}.

My ultimate goal was to achieve (at least) 100% reliability of the test - to eliminate all
race conditions. And the only way to do it is to do some kind of synchronization - using {{java.util.concurrent}}
primitives and listeners to BP / configadmin events.

So, when you add {{trace="{{tracing}}"}} to {{<camelContext>}}, then in 2.15.2 there's
great chance (unless you put a breakpoint to hold {{main}} thread), that BP container will
get initialized *after* [this|https://github.com/apache/camel/blob/camel-2.15.3/components/camel-test-blueprint/src/main/java/org/apache/camel/test/blueprint/CamelBlueprintTestSupport.java#L145-L150]:
{code:java}
if (file != null) {
  if (!new File(file[0]).exists()) {
    throw new IllegalArgumentException("The provided file \"" + file[0] + "\" from loadConfigAdminConfigurationFile
doesn't exist");
  }
  CamelBlueprintHelper.setPersistentFileForConfigAdmin(answer, file[1], file[0], props, symbolicName,
bpEvents, expectReload);
}
{code}
This leads to invocation of {{org.osgi.service.cm.Configuration#update(java.util.Dictionary<java.lang.String,
?>)}} and when (and there's high probability of this in 2.15.2) BP container isn't yet
started, it'll have those properties already available [when {{<cm:property-placeholder>}}
is initialized|https://github.com/apache/aries/blob/35c92e2c941f82db9c8815467e9173a5d710ab08/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedProperties.java#L140-L144]:
{code:java}
public void init() throws Exception {
    ...
    synchronized (lock) {
        managedObjectManager.register(this, props);
        Configuration config = CmUtils.getConfiguration(configAdmin, persistentId);
        if (config != null) {
            properties = config.getProperties();
        }
        updated(properties);
    }
}
{code}

Plus remember - there are two kinds of property placeholders: from Camel and from Aries-Blueprint.
Camel version delegates (in OSGi) to Aries version.

In order to have working {{trace="{{tracing}}"}} in 2.15.3+, you have to override simple method:
{{org.apache.camel.test.junit4.CamelTestSupport#useOverridePropertiesWithPropertiesComponent}}.

Add:
{code:java}
    @Override
    protected Properties useOverridePropertiesWithPropertiesComponent() {
        Properties props = new Properties();
        props.setProperty("tracing", "true");
        return props;
    }
{code}
to {{org.apache.camel.test.blueprint.ConfigAdminNoReloadLoadConfigurationFileTest}} and {{trace="{{tracing}}"}}
to {{org/apache/camel/test/blueprint/configadmin-no-reload-loadfile.xml}} and everything will
work, because PropertyComponent will have needed property when CamelContext is created.

In other words - my change in 2.15.3 related to predictability of camel-test-blueprint tests
can be summarized like this:
* we don't test (because of explicit synchronization) the scenarios, when configadmin configurations
are updated before blueprint container starts
* we test scenarios, where configadmin configuration updates should (when there's {{<cm:property-placeholder
... update-strategy="reload">}}) lead to reinitialization of blueprint container - and
to make it predictable, we have to synchrozize BP and configadmin events

It worked in 2.15.2 as a kind of side effect - just like the configadmin configuration already
existed when blueprint container was created for the first time. but it's not different then
specifying it explicitly with:
{code:xml}
  <cm:property-placeholder persistent-id="stuff" update-strategy="none">
    <cm:default-properties>
      <cm:property name="x" value="y"/>
    </cm:default-properties>
  </cm:property-placeholder>
{code}

And yet another words: {{CamelBlueprintTestSupport}} calls {{org.osgi.service.cm.Configuration#update(java.util.Dictionary<java.lang.String,?>)}}
*only* to change configadmin configs, not to init them.

I hope this resolves your issue.

> CamelBlueprintTestSupport - timing problems with property placeholder
> ---------------------------------------------------------------------
>
>                 Key: CAMEL-9313
>                 URL: https://issues.apache.org/jira/browse/CAMEL-9313
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-blueprint
>    Affects Versions: 2.15.4
>            Reporter: Andreas Siepert
>            Assignee: Grzegorz Grzybek
>             Fix For: 2.16.2, 2.15.5, 2.17.0
>
>
> The bugfix CAMEL-8948 seems to make older timing problems related to property-placeholders
visible.
> To reproduce the problem i changed the test 
> org.apache.camel.test.blueprint.ConfigAdminNoReloadLoadConfigurationFileTest 
> from the component camel-test-blueprint a bit, respectively the context 
> "org/apache/camel/test/blueprint/configadmin-no-reload-loadfile.xml": 
> I added the trace Attribute to the camelContext:
> {code:xml}
> <camelContext xmlns="http://camel.apache.org/schema/blueprint" trace="{{tracing}}">

> {code}
> and added also the property to the etc/stuff.cfg 
> {code}
> tracing=true 
> {code}
> Until 2.15.2 this worked fine. From 2.15.3 on the property cannot be 
> replaced any more.
> But, if setting a breakpoint in {{CamelBlueprintTestSupport#createBundleContext}} at
loadConfigAdminConfigurationFile() (Line 123 in 2.15.4) - the error occurs even in older versions
like 2.14 - so the timing problem seems to be there for a while but did not occur because
the loading of the configAdminFile seems to be faster than the event handling during service
registration triggered by the code some lines above.
> The issue can also be reproduced when replacing a property's String type 
> with int in the MyCoolBean class and setting its value by using the 
> placeholder like before but with an int value of course. The test run shows 
> that the placeholder "${..\}" will not be replaced and leads to a 
> NumberfFormatException. 
> The production code that is under test works fine in karaf. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message