Community
Participate
Working Groups
Currently the reconciler only checks for changes when Eclipse is started so any in-session changes to the drop-ins, links, or platform.xml file will not take effect until the next restart. We should discuss this and consider other possibilities. One idea John had was to listen to the PackageAdmin refresh package event and try to sync from there.
*** Bug 228028 has been marked as a duplicate of this bug. ***
Deferring to 3.5.
I don't think we want to start reconciling after startup at this point.
*** Bug 327872 has been marked as a duplicate of this bug. ***
Why don't you want to reconcile after startup? Are there any (performance) reasons or is it just a matter of priority? I think it would be very useful to have that functionality especially if you use dropins reconciler outside the Eclipse IDE. It's great that Eclipse is build by modular components and it would be even greater if they could be fine tuned. In case of dropins it could be easily implemented on top of OSGi configuration - a property defined by dropins could adjust the interval on which reconciling could be performed. If the configuration is missing, the default behavior would be used. So we would have 100% full backwards compatibility and the agility to handle the external use cases. What would you say about that approach?
I'm talking with Katya right now and she has a valid use case that is not involving the IDE but a server side app. We should consider this request. I think she will provide a patch.
Created attachment 184684 [details] Watcher for dropins on a configurable interval Watcher is implemented as ds component and is activated when: - ds and config admin are started - there is an appropriate configuration for dropins A service PID identifying the configuration of dropins is "org.eclipse.equinox.p2.reconciler.dropins" The configuration property "dropins.watch.interval" specifies the time, in milliseconds, between two consecutive checks for updated content in dropins folder. The value must be greater than zero, and less than or equal to Long.MaxValue If not set, check for dropins updates is performed only on bundle start.
(In reply to comment #5) > Why don't you want to reconcile after startup? Are there any (performance) > reasons or is it just a matter of priority? I think it would be very useful to > have that functionality especially if you use dropins reconciler outside the > Eclipse IDE. Sorry I missed this comment because I wasn't on the CC list. The issue isn't with installing software after startup, obviously we do that in the UI. The main issue is that the reconciler was only intended as a bridging technology to support the behaviour of the old update technology. We have been trying hard to get people to adopt proper p2 installation via the director. Some of the problems with the dropins: - It is "best effort" and does not fail if some things could not be installed. - All bundles installed via dropins are marked optional, so they will always lose when there is a conflict between a dropins bundle, and a bundle installed explicitly with p2 director. - Bundles installed via the reconciler don't provide p2 metadata. We need to compute that metadata on the fly at reconcile time. This means an IU installed this way can't take advantage of some of the more advanced capabilities available in p2 metadata. Also crafting this metadata on the fly is relatively expensive - The reconciler doesn't scale well in a large system. Resolution can take several minutes when there are many bundles in dropins. This is related both to metadata generation, and to the fact that they are optional (and therefore every time we need to evaluate whether to include them or not)
I went through the patch, here are some comments: - The DS component name should be fully qualified - There should be a p2.inf that turns the optional dependency on CM to be non greedy, since I don't think we should ship that enabled by default. - When the bundle is stopped, should the background reconciliation be stopped? - I'm wondering if the Thread.interrupt is a good way of stopping, since it looks like it could leave things in a undefined state that I think could cause problems. Instead I would think we would probably want to do this through a ProgressMonitor. Also we could probably use a job rather than a thread.
> - Bundles installed via the reconciler don't provide p2 metadata. We need to > compute that metadata on the fly at reconcile time. This means an IU installed > this way can't take advantage of some of the more advanced capabilities > available in p2 metadata. Also crafting this metadata on the fly is relatively > expensive IIRC it is possible to drop in a p2 repo in the runnable format.
Thanks for your comments; it's very useful to have a list with reconciler's pain points. We intend to use dropins' new feature in a very limited set of scenarios - mainly development. It's controlled environment and it's small scale. What we try to achieve is similar behavior to the pickup folder in Virgo. The main use case is to facilitate developer's life through the process of fixing and testing new functionality. So the best effort to install something is just fine since ideally there should not be a conflict with already installed bundles. The developer should be aware that such conflict may happen and should use another installation mechanism in that case. Operation failure could be easily detected by dropped bundle not installed. I agree that computing metadata on the fly could be expensive but it's the price for not performing manual step(i.e restarting the dropins). Again, it's only for development scenarios and it's not intended to be used productive. So, the new feature is not meant to be activated, nor the new dependency (Config admin) to be included in the SDK. In fact, from SDK point of view there should not be any difference in behavior or required bundles. I'm going to rewrite the patch to use jobs instead of thread, fully qualified ds name and p2.inf for making the ref to Config Admin non greedy. I'll write a performance test as well to be sure that the patch doesn't cause performance degradation.
Created attachment 187014 [details] Dropins watcher implemented with job instead of thread watcher service name changed to be fully qualified p2.inf added to make the dependency to configuration manager non greedy
The patch doesn't persist any additional information (compared to the original code), so no overhead when reading/writing file timestamps is expected. That's why I don't think there is a need of special performance test for the patch. Looking forward to your comments and remarks.
(In reply to comment #11) The usecase sounds cool and useful. This statement however concerns me. > Again, it's only for development scenarios and it's not intended to be used productive. In the context of this bug we can all agree that it is intended only for a few little tweaks but a few months from now someone else is going to notice this , turn it on and start deploying hundreds of bundles in production. I'm wondering if there is a way to expose the desired capability without this potential. Related, I could see something like this being useful but where it should be much lighter weight. For example, the current setup deals with platform.xml, publishes things, ... Both of these point to creating a new mechanism based more directly on the DirectoryWatcher and without all the reconciler cruft. People interested in this new mechanism could get/install it and get the precise behaviour it defines. We are free then to define exactly what it will do. As for the patch itself, the conversion of things to be statics and the addition of a new service that drives these statics feels like it sets up a conflict with the existing execution flows. Basically we have the same code being used for two incompatible purposes. It would be better to separate the scenarios and drive them independently.
> Both of these point to creating a new mechanism based more directly on the > DirectoryWatcher and without all the reconciler cruft. People interested in > this new mechanism could get/install it and get the precise behaviour it > defines. We are free then to define exactly what it will do. When I reviewed the patch, I wandered down this path. However given that we will still need some level of metadata generation (because we start from bundles), and that a *lot* of energy went in this code, I'm kind of reluctant to introduce something new but still doing more of the same. Also if we introduced a second mechanism, then we would open ourselves to having to be sure that the two mechanisms (the new and the old reconciler) cooperate to resolve dependencies that are split across the two mechanisms.
(In reply to comment #15) > When I reviewed the patch, I wandered down this path. However given that we > will still need some level of metadata generation (because we start from > bundles), and that a *lot* of energy went in this code, I'm kind of reluctant > to introduce something new but still doing more of the same. Agreed wrt duplicate code/effort avoidance. I was thinking that for the cases we are looking to address here the context can give us more help. For example, perhaps we don't have to publish anything and require the metadata be provided (e.g., you can only drop in repos or some such). Or, perhaps p2 is not involved at all and the directory watcher just calls installBundle() directly. Does that address the use case? My point is that we are hacking the all singing, all dancing, quite complex reconciler for some very specific case(s). This is how we have ended up with the various apparent disconnects in other parts of the code. Just sayin' > Also if we introduced a second mechanism, then we would open ourselves to > having to be sure that the two mechanisms (the new and the old reconciler) > cooperate to resolve dependencies that are split across the two mechanisms. Are there usecases for them to cohabitate? Are they mutually exclusive? I suspect they can/may/should be. Katya seems to say that their usecase does not involve this function in the IDE. It would be great to have something simple and small like the FileInstaller.
> Agreed wrt duplicate code/effort avoidance. I was thinking that for the cases > we are looking to address here the context can give us more help. For example, > perhaps we don't have to publish anything and require the metadata be provided > (e.g., you can only drop in repos or some such). > > Or, perhaps p2 is not involved at all and the directory watcher just calls > installBundle() directly. Does that address the use case? I agree that if installBundle is in the realm of possibilities then it should be used, and in this case FileInstaller or any variation of thereof be used. > This is how we have ended up with the various apparent disconnects in other parts of the code. I'm really interested in hearing you on this, probably on another venue though :) > Just sayin' > > > Also if we introduced a second mechanism, then we would open ourselves to > > having to be sure that the two mechanisms (the new and the old reconciler) > > cooperate to resolve dependencies that are split across the two mechanisms. > > Are there usecases for them to cohabitate? Are they mutually exclusive? I > suspect they can/may/should be. Katya seems to say that their usecase does not > involve this function in the IDE. I think this somewhat meet your initial concern that at some point someone will try to use the two together and will get burnt. If we were to create two mechanisms we would have to be sure that they can't be installed together. The other thing to consider if we were to go down the route of an osgi level dropin and a p2 level dropin, is that things installed at the OSGi level would not be available at the p2 level which would result in awkward dependency resolution problems. This could likely be alleviated by some sort of automagic generation of profiles or profiles additions, but this would not be pretty :)
(In reply to comment #14) > (In reply to comment #11) > > The usecase sounds cool and useful. This statement however concerns me. > > > Again, it's only for development scenarios and it's not intended to be used productive. > > In the context of this bug we can all agree that it is intended only for a few > little tweaks but a few months from now someone else is going to notice this , > turn it on and start deploying hundreds of bundles in production. > > I'm wondering if there is a way to expose the desired capability without this > potential. The functionality will not be enabled by default since the dependency on config admin will be marked non-greedy (comment #9 from Pascal). > Related, I could see something like this being useful but where it should be > much lighter weight. For example, the current setup deals with platform.xml, > publishes things, ... Some degree of publishing is necessary since we want user to be able to drop bundles. > Both of these point to creating a new mechanism based more directly on the > DirectoryWatcher and without all the reconciler cruft. People interested in > this new mechanism could get/install it and get the precise behaviour it > defines. We are free then to define exactly what it will do. > > As for the patch itself, the conversion of things to be statics and the > addition of a new service that drives these statics feels like it sets up a > conflict with the existing execution flows. Basically we have the same code > being used for two incompatible purposes. It would be better to separate the > scenarios and drive them independently. I introduced the statics in order to minimize the code change and ease the adoption of the patch. If this is fine with the committer in charge of this area, I'm happy to create another patch that will remove the statics. Just to mention that my intention is not to introduce new mechanism but to use the existing one iteratively. Currently on dropins start we have initialization step, then synchronization, then cleanup. Since the patch suggests repetition of that code several times I thought it's reasonable to have initialization and cleanup steps only once and iterating just over the synchronization step. That's basically the whole patch.
(In reply to comment #18) > The functionality will not be enabled by default since the dependency on config > admin will be marked non-greedy (comment #9 from Pascal). What would happen if config admin were installed by some other dependency? Would the reconciler start polling? > Some degree of publishing is necessary since we want user to be able to drop > bundles. Just to clarify the scenario, I imagined it to be something where developers start say a server and then incrementally deploy bundles as they implement or test new function. On the surface these new bundles do not need to be in a p2 profile etc so no metadata is needed. I could see that in a bigger context they may need to be known by p2 but that has not been expressed in the scenario. FWIW, this feels a lot like the live launching work we have fiddled with in the past. Perhaps you can say more about the scenario. > I introduced the statics in order to minimize the code change and ease the > adoption of the patch. If this is fine with the committer in charge of this > area, I'm happy to create another patch that will remove the statics. Dunno who is in charge of that code per se but we have been systematically moving away from statics and singletons. Every time we use them we eventually get burnt or limited. So I'd be happy to see something that does not involve adding more and hopefully even removes some of this assumptions. > Just to mention that my intention is not to introduce new mechanism but to use > the existing one iteratively. Currently on dropins start we have initialization > step, then synchronization, then cleanup. Since the patch suggests repetition > of that code several times I thought it's reasonable to have initialization and > cleanup steps only once and iterating just over the synchronization step. > That's basically the whole patch. Understood. I'm all for incrementally improving/extending existing mechanisms. We do have to be aware however that these existing mechanisms are in use so enhancing in mutually exclusive ways or making use-case assumptions may well conflict with user expectations and assumptions. All it will take is one bright spark to blog how to turn this on and we'll be stuck with all the problems we were trying to avoid when we made the explicit choice not to poll. Sometimes its better do just do something simple to solve the simple problems.
(In reply to comment #19) > (In reply to comment #18) > > The functionality will not be enabled by default since the dependency on config > > admin will be marked non-greedy (comment #9 from Pascal). > > What would happen if config admin were installed by some other dependency? > Would the reconciler start polling? Configuration admin presence is not enough - you need to have a proper configuration available to start polling. Configuration should: - correspond to service.pid= org.eclipse.equinox.p2.reconciler.dropins - contain property dropins.watch.interval I think it's difficult to activate recurring check for new content unintentionally but if you have concerns we can discuss them. > > Some degree of publishing is necessary since we want user to be able to drop > > bundles. > > Just to clarify the scenario, I imagined it to be something where developers > start say a server and then incrementally deploy bundles as they implement or > test new function. On the surface these new bundles do not need to be in a p2 > profile etc so no metadata is needed. I could see that in a bigger context > they may need to be known by p2 but that has not been expressed in the > scenario. > > FWIW, this feels a lot like the live launching work we have fiddled with in the > past. > > Perhaps you can say more about the scenario. Not generating metadata would work fine if you need to test just one bundle for example. Imagine you have a relatively complex component and you want to test each of its modules before testing how they integrate together. So you 1. test a module 2. find bugs 3. fix them 4. finally get it working 5. update the system with this module (p2 update) 6. move to another module (step 1.) Proposed change in dropins makes this easier from developer's point of view. Dropins saves you time and effort needed to install bundles by hand, to figure out the dependencies on your own and finally to install your module via p2 in order to have your system "legally" updated. I agree the steps above are simple but performing them many times (the normal case during development) focuses the developer on infrastructure rather than on module validation. From my point of view every optimization in the process would increase p2 usability. This is completely a development scenario and that's why I said it's not expected to be used productive. But I see your point - when there is a mechanism available it's difficult to restrict its usages. What I propose in order to make the user aware of all the issues you mentioned is to choose the poll policy explicitly. In order to activate recurring polling the user should provide a specific configuration. This could be treated as service level agreement stating that the user understands all the limitations of this approach. > > I introduced the statics in order to minimize the code change and ease the > > adoption of the patch. If this is fine with the committer in charge of this > > area, I'm happy to create another patch that will remove the statics. > > Dunno who is in charge of that code per se but we have been systematically > moving away from statics and singletons. Every time we use them we eventually > get burnt or limited. So I'd be happy to see something that does not involve > adding more and hopefully even removes some of this assumptions. Completely agree with you. New patch proposed. > > Just to mention that my intention is not to introduce new mechanism but to use > > the existing one iteratively. Currently on dropins start we have initialization > > step, then synchronization, then cleanup. Since the patch suggests repetition > > of that code several times I thought it's reasonable to have initialization and > > cleanup steps only once and iterating just over the synchronization step. > > That's basically the whole patch. > > Understood. I'm all for incrementally improving/extending existing mechanisms. > We do have to be aware however that these existing mechanisms are in use so > enhancing in mutually exclusive ways or making use-case assumptions may well > conflict with user expectations and assumptions. All it will take is one bright > spark to blog how to turn this on and we'll be stuck with all the problems we > were trying to avoid when we made the explicit choice not to poll. Sometimes > its better do just do something simple to solve the simple problems.
Created attachment 189951 [details] Dropins watcher as job and no statics
