Bugzilla – Full Text Bug Listing
| Summary: | [planner] Optimization function picks up unnecessary IUs | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Pascal Rapicault <pascal> | ||||||||||
| Component: | p2 | Assignee: | P2 Inbox <equinox.p2-inbox> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | bokowski, kim.moir, leberre, ob1.eclipse, pwebster, simon_kaegi, slewis, stephan.herrmann, tjwatson | ||||||||||
| Version: | 3.5 | Flags: | pascal:
review+
|
||||||||||
| Target Milestone: | 3.5 RC2 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 277161 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Pascal Rapicault
Created attachment 135643 [details]
Tentative patch
This patch allows the NonMinimalState tests to pass but cause other to fail. The failing tests may be bogus tests but I need to check.
I also need to verify why mx4j was not part of the solution before the fix is applied and what would have happened if there had been 2 versions of tptp.
There is another, maybe simpler, solution that could be applied in SAT4J to solve that problem, namely to ensure that SAT4J finds first a solution without extra IUs. This can be done using a specific heuristic that will look first for a solution without a given IU (i.e. always branch first on a negative literal technically speaking). It is just a specific configuration of the SAT solver used for P2. No changes would be required on P2 side. *** Bug 276557 has been marked as a duplicate of this bug. *** Created attachment 136104 [details]
Patch fixing the problem in the SimplePlanner
This patch implements an idea from John which consists in re-running the solver once we have the solution.
Created attachment 136107 [details]
Contextualization of the optionality constraint
Here is an alternative solution, that will not have the drawback to call twice the solver.
The optional IUs are forced to be installed currently because the abstract variables used to create the optional expressions
Abstptp-admin -> tptp-admin-vXXX
have a hig coefficient in the objective function.
It is sufficient to contextualize that constraint with the IU for which that IU is an optional dependency, i.e.
Abstptp-admin /\ hyades -> tptp-admin-vXXX
That way, even if the abstract variable is set to true, the installation of the optional plugin is only fired if the main IU is installed.
There is one test failing with that patch:
AllTests
org.eclipse.equinox.p2.tests.planner.AllTests
org.eclipse.equinox.p2.tests.planner.MissingOptionalWithDependencies3
testInstallation(org.eclipse.equinox.p2.tests.planner.MissingOptionalWithDependencies3)
junit.framework.AssertionFailedError: Can't find B 1.0.0 in the plan
at junit.framework.Assert.fail(Assert.java:47)
at org.eclipse.equinox.p2.tests.AbstractProvisioningTest.assertInstallOperand(AbstractProvisioningTest.java:835)
at org.eclipse.equinox.p2.tests.planner.MissingOptionalWithDependencies3.testInstallation(MissingOptionalWithDependencies3.java:59)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at junit.framework.TestCase.runTest(TestCase.java:164)
at org.eclipse.equinox.p2.tests.AbstractProvisioningTest.runTest(AbstractProvisioningTest.java:763)
at junit.framework.TestCase.runBare(TestCase.java:130)
at junit.framework.TestResult$1.protect(TestResult.java:106)
at junit.framework.TestResult.runProtected(TestResult.java:124)
at junit.framework.TestResult.run(TestResult.java:109)
at junit.framework.TestCase.run(TestCase.java:120)
at junit.framework.TestSuite.runTest(TestSuite.java:230)
at junit.framework.TestSuite.run(TestSuite.java:225)
at junit.framework.TestSuite.runTest(TestSuite.java:230)
at junit.framework.TestSuite.run(TestSuite.java:225)
at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62)
at org.eclipse.pde.internal.junit.runtime.CoreTestApplication.run(CoreTestApplication.java:23)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.eclipse.equinox.internal.app.EclipseAppContainer.callMethodWithException(EclipseAppContainer.java:574)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:559)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:514)
at org.eclipse.equinox.launcher.Main.run(Main.java:1311)
at org.eclipse.equinox.launcher.Main.main(Main.java:1287)
I'm running with all the p2 code from HEAD + only Daniel's patch and all the tests are passing. I'm using SAT4J 2.1.0_v20090513 (the one included in the most recent I build) Created attachment 136141 [details]
Feature patch for RC1
Here is a binary version of the director plug-in including Daniel's patch. To install it, download the archive, point p2 at the archive and install the feature patch listed.
If you don't uncheck "contact all update sites..." installing the feature patch will install the unwanted tptp plugins (at least on my machine) PW (In reply to comment #8) > If you don't uncheck "contact all update sites..." installing the feature patch > will install the unwanted tptp plugins (at least on my machine) > > PW > I assume that expected since we would be installing the feature patch with the old code with the bug to puck up unnecessary IUs, correct? If you mean that you took a fresh RC1, installed the feature patch and got tptp at the end, this is "normal" in the sense that the feature patch has the objective to fix that issue. Now that the feature patch is installed, you should be able to install anything with the "contact all the update sites" checked without having undesirable plugins installed. (In reply to comment #10) > If you mean that you took a fresh RC1, installed the feature patch and got tptp > at the end, this is "normal" in the sense that the feature patch has the > objective to fix that issue. Yes :-) Just a note (since it would invalidate testing for some of us that have this problem) PW With the feature patch on I20090518-2000, my updates no longer includes the spurious tptp plugins. PW I see the same single test failure as Daniel when running with the patch on RC1 (MissingOptionalWithDependencies3). John could you detail which VM / OS combination you are seeing this? Daniel did you verify that we were not hitting a time out? >John could you detail which VM / OS combination you are seeing this?
Windows XP, IBM Java 5 sr4.
To comment #14: We cannot hit a timeout here, the problem is too simple. For me, it is a valid limit of the current optimization function: if IU A depends optionally on B, and B depends optionally on C, and C cannot be found, the value of the objective function will be greater if B is not installed (if the dependency was not optional, then B would be forced to be installed). I haven't found a workaround yet. But I believe that such test case will not appear that frequently, while we are hitting the wall each time an update is done using RC1 objective function :( Don't get me wrong, we will release the patch you put it. This is sure. However I want to understand why on some machines we pass and on others we fail where I was expecting this process to deterministic. Same for me. The process is not deterministic because the order in which we create the constraints is not deterministic. However, I do not believe that the problem we have here is limited to that order. I have an important meeting now. I will be back in a few hours. I have reviewed and released this bug. I have left the MissingOptionalWithDependencies3 test enabled for now. Here is the explanation why some of us pass the test an others do not. There are too equally good solutions according to the optimization function (with a value of -7): Abs_B 1.0.0,A 1.0.0,D 1.0.0,C 2.0.0,Noop_D 1.0.0,1242834377753 0.0.0.1242834377753 B 1.0.0,A 1.0.0,Noop_B 1.0.0,D 1.0.0,Abs_D 1.0.0,C 2.0.0,1242834377753 0.0.0.1242834377753 It is important to note that the rewards are given on Abs_X variables and penalties are given on Noop_X variables. Since the two solutions have the same number of Abs_ variables and Noop_X variables, they are equivalent to the solver. The one that comes first depend on the order in which the constraints are entered in the solver. Since the order of the IUs depends basically on their address in memory, you can understand that such order might change depending on the VM or the Closing as fixed. I have disabled the test intermittently failing. When I try to install a trivial addition: feature with one new plugin, I also get the following installed: slf4j.api slf4j.jcl org.mortbay.management This is on I20090520-2000, using sample from the bug 276804. This is because you the dependencies from jetty to those bundle is optional and greedy, and as such we always install them since we try to satisfy as many optional pieces as possible. That said, the metadata of jetty has been changed to have these optional dependencies be non greedy and as such it should not bring slf4j in. *** Bug 276718 has been marked as a duplicate of this bug. *** |