felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Jencks (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (FELIX-5435) [DS] Service does not get loaded with updated properties that have been modified on file system after felix framework restart
Date Wed, 21 Dec 2016 17:48:58 GMT

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

David Jencks edited comment on FELIX-5435 at 12/21/16 5:48 PM:
---------------------------------------------------------------

I thought it was explained pretty clearly in FELIX-5270, but here's a simplified version.

1. management agent creates configuration with properties.

2. DS detects receives configuration event.

3. management agent deletes configuration.

4. DS calls getConfiguration, creating a new configuration with null properties.

At this point, DS can't notice that the properties are null and delete the configuration it
just accidentally created, because

5. management agent sets properties on the configuration

6 DS deletes the configuration....oops

There are (at least theoretically) two possible strategies for writing a DS implementation's
configuration-awareness: ManagedService + ManagedServiceFactory or configuration events. 
Use of the MS+MSF strategy will result in CA setting the bundle location (if missing) for
any configuration consumed by DS.  As seen above, use of the ConfigurationEvent strategy requires
DS to not ever try to set the bundle location.  Therefore the expert group decided, in order
to allow both these strategies, to not take a position on whether DS consuming a configuration
should result in the bundle location being set (if missing).  Personally I don't know how
to write a MS+MSF based strategy and think it would be horribly inefficient.


was (Author: djencks):
I thought it was explained pretty clearly in FELIX-5270, but here's a simplified version.

1. management agent creates configura

> [DS] Service does not get loaded with updated properties that have been modified on file
system after felix framework restart
> -----------------------------------------------------------------------------------------------------------------------------
>
>                 Key: FELIX-5435
>                 URL: https://issues.apache.org/jira/browse/FELIX-5435
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>    Affects Versions: scr-2.0.4, scr-2.0.6
>         Environment: Java 1.7; Using Felix framework version 5.4.0. 
>            Reporter: Alin Brici
>            Priority: Blocker
>
> In felix.scr 2.0.6, seeing an issue where a configuration for a particular Service is
not loaded. 
> We have one @Component(Componenent A) that offers one service, lets call this Service
A. 
> ComponenetA attributes are as follows: {noformat}(name="ServiceA", policy= ConfigurationPolicy.REQUIRE,
metatype=true, immediate=true) {noformat}
> ServiceA has a configuration that could look like this:
> {noformat}
> {
> 	"name" : "ServiceA",
> 	"description" : "Displays which version it has of ServiceB",
> 	"versionRequired" : "1"
> }
> {noformat}
> We have a second @Component(Component B) that offers another service, lets call that
Service B. 
> The Component attributes for Component B are as follows: {noformat}(name:"ServiceB",
policy=ConfigurationPolicy.OPTIONAL, metatype=true, immediate=true).{noformat}
> (I understand with immediate=true and ConfiguraitonPolicy.OPTIONAL, Service B will be
initialized without any configuration when it first gets started by the framework, however
if there is configuration for it on the file system, Felix DS will reload Service B with what
is on file system when it scans the directory for ServiceB configuraiton). 
> Service B, has some properties in the configuraiton for it, that are used by ServiceA
if ServiceB has been loaded with configuration. For example say we have a configuration for
ServiceB that looks like this. 
> {noformat}
> {
> 	"name" : "ServiceB",
> 	"description" : "is used by ServiceA"
> 	"version" : "1"
> }
> {noformat}
> Service A has a reference to Service B, as so:
> {noformat}
> @Reference(policy = ReferencePolicy.DYNAMIC)
> volatile ServiceB serviceB = null;
> For example if we have something like this for ComponentA/ServiceA, I will keep the code
short to illustrate the problem. 
> 	@Component(name = "ServiceA",
>         policy = ConfigurationPolicy.REQUIRE,
>         metatype = true,
>         description = "ComponentA with ServiceA",
>         immediate = true)
> 	@Service(value = {ServiceAInterface.class})
> 	public class ServiceA implements ServiceAInterface {
> 		private String version; 
> 		private String description; 
> 		private Json config = null;
> 		// version requested by VersionB 
> 		private String versionRequested;
> 		@Reference(policy = ReferencePolicy.DYNAMIC)
> 		volatile ServiceB serviceB;
> 		@Activate
> 		protected void activate(ComponentContext context) {
> 			this.config = getConfigFromComponentContext(context);
> 			this.description = config.get("description");
> 			this.versionRequested = config.get("versionRequested");
> 			// get the version that ServiceA config requests 
> 			serviceB.getVersionAsStringAsync(versionRequested).thenOnResult(
> 				this.version = versionFromServiceB; 
> 			);
> 		}
> 		// @Deactivate method would go here
> 		@override
> 		public String getVersion() {
> 			if (version != null) {
> 				return version;
> 			} else {
> 				return "No Version has been set yet!";
> 			}
> 		}		
> 	}
> {noformat}
> For the scenario where this problem exists we need a configuration for both ServiceA
and ServiceB. The configuration is stored on the file system in some directory that felix
watches to load changes as they occur. /tmp/felixConfig/ ServiceA.json ServiceB.json.   
> Start up the Felix OSGi environment with the configuration and what I see is that it
works as expected. ServiceA waits until it has its @Refernce to ServiceB satisfied before
it attempts to activate as expected. Then if I made a call to ServiceA#getVersion("1"), and
ServiceB is up and running with configuration from the file that was found, ServiceA will
be able to get the version. 
> Here is where the problem is. I shutdown the OSGi Framework. Then I change the configuration
to both ServiceA and ServiceB, "versionRequired" : "2" and "version" : "2", respectively.
I then start the OSGi framework back up and now ServiceA does not load correctly in the ComponentRegistry
as "versionRequired" : "2", but rather has the old version, "1". I see that felix.scr picked
up the changes and is attempting to notify all the ConfigurationListener(org.osgi.service.cm.ConfigurationListener)
implementations. 
> The setup I am working with picks up a change and makes a call to org.apache.felix.cm.impl.ConfigurationAdapter#update,

> {noformat}
> 	@Override 
>     public void update( Dictionary<String, ?> properties ) throws IOException
>     {
>         delegatee.getConfigurationManager().log( LogService.LOG_DEBUG, "update(properties={0})",
new Object[]
>             { properties } );
>         checkActive();
>         checkDeleted();
>         delegatee.update( properties );
>     }
> {noformat}   
> {noformat}
> ** An Important NOTE **
> I set a breakpoint at `delegatee.update(properties)`. When I look here at the `configurations`
inside the ConfigurationAdapter->ConfigurationAdmin->configurations, the configuration
for ServiceA is not here! The configuration for ServiceB is in that list of configurations.
IF I look at a previous version of felix.scr, 2.0.2, I see ServiceA in that list of configurations..it
no longer is there, in version 2.0.4,2.0.6, and trunk and this will result in the problem
because the revision number will not be updated.
> **      **
> {noformat}    
> These properties are the new properties for ServiceA, that has the the versionRequested
attribute in the ServiceA object updated to versionRequest = 2, because I changed it on the
file system when the system was down and on startup of the framework it pick up those changes
on the file system and is attempting to propagate this configuration to all necessary services
that require it. 
> Digging deeper in the call to delegatee.update( properties), is implemented in org.apache.felix.cm.impl#update(Dictionary<String,
?> properties) as so, 
> {noformat}
>   * @see org.osgi.service.cm.Configuration#update(java.util.Dictionary)
>      */
>     public void update( Dictionary<String, ?> properties ) throws IOException
>     {
>         PersistenceManager localPersistenceManager = getPersistenceManager();
>         if ( localPersistenceManager != null )
>         {
>             CaseInsensitiveDictionary newProperties = new CaseInsensitiveDictionary(
properties );
>             getConfigurationManager().log( LogService.LOG_DEBUG, "Updating config {0}
with {1}", new Object[]
>                 { getPidString(), newProperties } );
>             setAutoProperties( newProperties, true );
>             // persist new configuration
>             localPersistenceManager.store( getPidString(), newProperties );
>             // finally assign the configuration for use
>             configure( newProperties );
>             // if this is a factory configuration, update the factory with
>             // do this only after configuring with current properties such
>             // that a concurrently registered ManagedServiceFactory service
>             // does not receive a new/unusable configuration
>             updateFactory();
>             // update the service and fire an CM_UPDATED event
>             getConfigurationManager().updated( this, true );
>         }
>     }
> {noformat}
> The intresting thing here is that the reference now to 'this' ConfigurationImpl seen
on the code above `getConfigurationManager().updated( this, true );`
> has the new revision, and properties of the ConfigurationImpl, but later on when we dig
deeper in the call to getConfigurationManager().updated( this, true ); 
> we will see that we cannot find the ConfigurationImpl(with the UPDATED revision and properties)
in the list of Configurations of the ConfirurationAdmin.
> Following the getConfigurationManager().updated( this, true ); code brings us to where
the ConfigurationManager perpares the eventTread and the UpdateThread as so:
> org.apache.felix.cm.impl.ConfigurationManager
> {noformat}
> 	void updated( ConfigurationImpl config, boolean fireEvent )
>     {
>         if ( fireEvent )
>         {
>             fireConfigurationEvent( ConfigurationEvent.CM_UPDATED, config.getPidString(),
config.getFactoryPidString() );
>         }
>         updateThread.schedule( new UpdateConfiguration( config ) ); // this will occur
with the correct revision...
>         log( LogService.LOG_DEBUG, "UpdateConfiguration({0}) scheduled", new Object[]
>             { config.getPid() } );
>     }
> {noformat}
> One very important thing to note here is that the `ConfigurationImpl config` passed into
this updated(config, fireEvent) method has the correct `revision` and `properties`. 
> That ConfigurationImpl is not passed into the `fireConfigurationEvent`, but we later
look it up in the ConfigAdmin...which we will see that it is not there, and this is what I
believe to be the root of the problem.
> Before we get to the updateThread.schedule(), lets go ahead into the fireConfiguraitonEvent()
which creates a new FireConfigurationEvent object for both asyncSender and for 
> syncSender, I did not have any syncSender so that could be ignored. This new object gets
scheduled in the `eventThread`. This thread will then loop through each ConfigurationListener
> and passes the ConfiguraitonEvent that it had created letting them know about the changed.

> Using from the sample I wrote above, ComponentA and all its properties are part of this
ConfigurationEvent. The listener that is having problems actually doing anything with the
ConfigurationEvent is the 
> org.apache.felix.scr.impl.manager.RegionConfigurationSupport#configurationEvent(ConfigurationEvent
event) 
> when we look at the code inside the implementation of that we see this line that is returning

> a ConfigurationInfo that has a revision that is revision=1, and the properties are the
updatedProperties{version=2,etc.} instead of returning revision=2,updatedProperties{version=2,etc.}
> around like 259
> {noformat}
> final ConfigurationInfo configInfo = getConfigurationInfo( pid, targetedPid, // This
IS where the config returns the wrong revision
>                                 componentHolder, bundleContext );
> {noformat}
> If we dig deeper to understand why the ConfigurationInfo comes back with the correct
updated properties but without updating the revision to 2 we will inside 
> org.apache.felix.scr.impl.manager.RegionConfigurationSupport#getConfigurationInfo(final
TargetedPID pid, TargetedPID targetedPid, ComponentHolder<?> componentHolder, final
BundleContext bundleContext)
> {noformat}
>     try
>         {//final ServiceReference caRef = bundleContext.getServiceReference(ComponentRegistry.CONFIGURATION_ADMIN);
>             final ConfigurationAdmin ca = getConfigAdmin( bundleContext );
>             try  // this looks interesting, could be getting the wrong config admin?
?
>             {
>                 Configuration[] configs = ca.listConfigurations( filter( pid.getRawPid()
) ); <----- this ca.listConfigurations() is the problem, like I mentioned earlier in the
** NOTE **
>                 if ( configs != null && configs.length > 0 )
>                 {
>                     Configuration config = configs[0];
>                     return new ConfigurationInfo( config.getProperties(), config.getBundleLocation(),

>                         config.getChangeCount() );
>                 }
>                 ... the rest of the implementation
> {noformat}
> The problem I am seeing is the the `ConfigurationAdmin ca` does not have the configuration
in its cachedConfigs
> {noformat}
>      // the cache of Configuration instances mapped by their PID
>     // have this always set to prevent NPE on bundle shutdown
>     private final HashMap<String, ConfigurationImpl> configurations = new HashMap<String,
ConfigurationImpl>();
> {noformat}
> when it makese the call to ca.listConfigurations..
> {noformat}
>  				// ensure the service.pid and returned a cached config if available
>                 ConfigurationImpl cfg = getCachedConfiguration( pid ); <----- this
returns null
>                 if ( cfg == null )
>                 {
>                     cfg = new ConfigurationImpl( this, pmList[i], config ); 
>                 }
> {noformat}
> it returns no configuraiton from the cache, so then we return a new ConfigurationImpl
which configures a NEW implementation of `ConfigurationImpl` from what we stored in the PersistanceManager
above
> at org.apache.felix.cm.impl#update(Dictionary<String, ?> properties), but the revision,
because it's a NEW object, is set to 1. and when this goes back up the stack and gets to 
> org.apache.felix.scr.impl.manager.ConfigurableComponentHolder#configurationUpdated()
> The logical statement inside,
> {noformat}
>                 if (oldChangeCount != null && changeCount <= oldChangeCount
&& factoryPid.equals(oldTargetedPID)) {
>                 	return false;
>                 }
> {noformat}
> is TRUE so the code immediately returns false and the Component(ComponentA) is not reconfigured
with the new properties. 
> This DOES NOT happen on version felix.scr 2.0.2, but tracing through that code I see
a lot of code that has been refactored. The RegionConfigurationSupport has taken over what
was the ConfigurationSupport. The revision changeCount has been refactored, and the CA has
usage has also been refactored. 
> It is broken in both 2.0.4, and 2.0.6. 
> I did a manual bisect until I was able to narrow it down, here is a snapshot of when
the behavior was introduced: 
> {noformat}
> ------------------------------------------------------------------------
> r1747831 | djencks | 2016-06-10 18:14:36 -0700 (Fri, 10 Jun 2016) | 1 line   <-------------
TEST HERE [   BROKEN  ]
> FELIX-5270 Don't set bundle location on configurations 
> ------------------------------------------------------------------------
> {noformat}
> This only started occuring after the first commit to {noformat}FELIX-5270{noformat}
> Here a larger snapshot of the actual bisect steps I took: 
> {noformat}
> FELIX-5270 log when bundle locations are inconsistent
> ------------------------------------------------------------------------
> r1749929 | djencks | 2016-06-23 08:57:30 -0700 (Thu, 23 Jun 2016) | 1 line   <-------------
TEST HERE [   BROKEN  ] AT THIS REVISION 
> FELIX-5248 missing license header
> ------------------------------------------------------------------------
> r1749927 | cziegeler | 2016-06-23 08:48:00 -0700 (Thu, 23 Jun 2016) | 1 line
> Revert rev 1749869
> ------------------------------------------------------------------------
> r1749869 | cziegeler | 2016-06-23 04:45:33 -0700 (Thu, 23 Jun 2016) | 1 line 
> add missing license header
> ------------------------------------------------------------------------
> r1748287 | djencks | 2016-06-13 10:14:22 -0700 (Mon, 13 Jun 2016) | 1 line  
> FELIX-5248 test for complaint
> ------------------------------------------------------------------------
> r1747831 | djencks | 2016-06-10 18:14:36 -0700 (Fri, 10 Jun 2016) | 1 line   <-------------
TEST HERE [   BROKEN  ] WINNER!!!
> FELIX-5270 Don't set bundle location on configurations 						
> ------------------------------------------------------------------------
> r1747329 | djencks | 2016-06-07 17:14:12 -0700 (Tue, 07 Jun 2016) | 1 line   <-------------
TEST HERE [ WORKS ]
> FELIX-5276 track service event before changing service properties
> ------------------------------------------------------------------------
> r1746618 | djencks | 2016-06-02 12:36:26 -0700 (Thu, 02 Jun 2016) | 1 line    <-------------
TEST HERE [   WORKS   ]
> FELIX-4417 Improve logging of circular references.  Fix some problems introduced with
rev 1744827 when activate changes service properties.
> ------------------------------------------------------------------------
> r1746617 | djencks | 2016-06-02 12:36:22 -0700 (Thu, 02 Jun 2016) | 1 line    
> FELIX-5264 Introduce a single State enum and use an atomic to track it, and use some
optimistic locking on state changes.  This fixes the specific issue found and should provide
much easier diagnosis of any remaining or new problems.
> ------------------------------------------------------------------------
> r1746471 | gnodet | 2016-06-01 07:36:32 -0700 (Wed, 01 Jun 2016) | 1 line <--------
TEST HERE [ WORKS ]
> [FELIX-5243] Make ComponentContextImpl#setImplementationAccessible public, similar to
setImplementationObject
> ------------------------------------------------------------------------
> r1746470 | gnodet | 2016-06-01 07:20:03 -0700 (Wed, 01 Jun 2016) | 1 line
> [FELIX-5243] Remove anonymous inner class, add a unit test to ensure package consistency
> {noformat}



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

Mime
View raw message