You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2017/04/06 14:14:29 UTC
[GitHub] brooklyn-server pull request #622: Integration tests fixes
GitHub user neykov opened a pull request:
https://github.com/apache/brooklyn-server/pull/622
Integration tests fixes
Either fixes or marks integration tests as Broken so the CI integration test runs reliably.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/neykov/brooklyn-server fix/integration-tests-3
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/622.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #622
----
commit 30388c171c351273f40ea771617d96279aa1f9ad
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2017-04-06T14:07:41Z
Fixes integration test failures
Or disables failing tests which can't be fixed now.
commit 49749488e91cb50422a0c0881c80bfef7e78bc2c
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2017-04-06T14:11:17Z
Adds tests which trigger the root cause leading to failures in testChildFailuresOnStartButWithQuorumCausesAppToSucceed
The tests are indeterministic - fail a couple of times per 100 invocations when executing with "mvn test" in the brooklyn-itest docker container.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #622: Integration tests fixes
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/622
@neykov I ran this locally - it hung. Doing `jstack` showed a deadlock! Looks like this is caused by the synchronization changes in `AttributeMap.update`.
```
Found one Java-level deadlock:
=============================
"brooklyn-execmanager-P37YeS8X-2":
waiting to lock monitor 0x00007ff7f7c8be98 (object 0x00000007b482f468, a org.apache.brooklyn.core.mgmt.internal.EntityManagementSupport),
which is held by "main"
"main":
waiting to lock monitor 0x00007ff7f847fe08 (object 0x00000007b482f880, a java.util.Collections$SynchronizedMap),
which is held by "brooklyn-execmanager-P37YeS8X-2"
Java stack information for the threads listed above:
===================================================
"brooklyn-execmanager-P37YeS8X-2":
at org.apache.brooklyn.core.mgmt.internal.EntityManagementSupport.getSubscriptionContext(EntityManagementSupport.java:363)
- waiting to lock <0x00000007b482f468> (a org.apache.brooklyn.core.mgmt.internal.EntityManagementSupport)
at org.apache.brooklyn.core.entity.AbstractEntity$BasicSubscriptionSupport.getSubscriptionContext(AbstractEntity.java:1431)
at org.apache.brooklyn.core.entity.AbstractEntity$BasicSensorSupport.emitInternal(AbstractEntity.java:1215)
at org.apache.brooklyn.core.entity.AbstractEntity.emitInternal(AbstractEntity.java:1996)
at org.apache.brooklyn.core.sensor.AttributeMap.update(AttributeMap.java:136)
- locked <0x00000007b482f880> (a java.util.Collections$SynchronizedMap)
at org.apache.brooklyn.core.entity.AbstractEntity$BasicSensorSupport.set(AbstractEntity.java:1114)
at org.apache.brooklyn.core.enricher.AbstractEnricher.emit(AbstractEnricher.java:137)
at org.apache.brooklyn.enricher.stock.AbstractTransformer.onEvent(AbstractTransformer.java:148)
at org.apache.brooklyn.core.mgmt.internal.LocalSubscriptionManager$1.run(LocalSubscriptionManager.java:276)
at org.apache.brooklyn.util.concurrent.CallableFromRunnable.call(CallableFromRunnable.java:44)
at org.apache.brooklyn.util.core.task.BasicExecutionManager$SubmissionCallable.call(BasicExecutionManager.java:529)
at org.apache.brooklyn.util.core.task.SingleThreadedScheduler$1.call(SingleThreadedScheduler.java:116)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
"main":
at java.util.Collections$SynchronizedMap.get(Collections.java:2584)
- waiting to lock <0x00000007b482f880> (a java.util.Collections$SynchronizedMap)
at org.apache.brooklyn.core.sensor.AttributeMap.getValue(AttributeMap.java:204)
at org.apache.brooklyn.core.sensor.AttributeMap.getValue(AttributeMap.java:210)
at org.apache.brooklyn.core.entity.AbstractEntity$BasicSensorSupport.get(AbstractEntity.java:1089)
at org.apache.brooklyn.core.entity.AbstractEntity.getAttribute(AbstractEntity.java:996)
at org.apache.brooklyn.core.mgmt.internal.LocalSubscriptionManager.subscribe(LocalSubscriptionManager.java:147)
- locked <0x00000007b4777818> (a org.apache.brooklyn.core.mgmt.internal.LocalSubscriptionManager)
at org.apache.brooklyn.core.mgmt.internal.QueueingSubscriptionManager.startDelegatingForSubscribing(QueueingSubscriptionManager.java:81)
- locked <0x00000007b482f518> (a org.apache.brooklyn.core.mgmt.internal.QueueingSubscriptionManager)
at org.apache.brooklyn.core.mgmt.internal.EntityManagementSupport.onManagementStarting(EntityManagementSupport.java:186)
- locked <0x00000007b482f468> (a org.apache.brooklyn.core.mgmt.internal.EntityManagementSupport)
at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager$2.apply(LocalEntityManager.java:392)
at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager$2.apply(LocalEntityManager.java:348)
at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.recursively(LocalEntityManager.java:619)
at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.manageRecursive(LocalEntityManager.java:413)
at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.manage(LocalEntityManager.java:307)
at org.apache.brooklyn.core.mgmt.internal.LocalEntityManager.createEntity(LocalEntityManager.java:153)
at org.apache.brooklyn.core.entity.AbstractEntity.addChild(AbstractEntity.java:710)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.apache.brooklyn.core.objs.proxy.EntityProxyImpl.invoke(EntityProxyImpl.java:201)
at com.sun.proxy.$Proxy198.addChild(Unknown Source)
at org.apache.brooklyn.entity.software.base.EntitySshToolTest.testCustomSshToolClassConfiguredOnBrooklynProperties(EntitySshToolTest.java:85)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:104)
at org.testng.internal.Invoker.invokeMethod(Invoker.java:645)
at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:851)
at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1177)
at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
at org.testng.TestRunner.privateRun(TestRunner.java:756)
at org.testng.TestRunner.run(TestRunner.java:610)
at org.testng.SuiteRunner.runTest(SuiteRunner.java:387)
at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:382)
at org.testng.SuiteRunner.privateRun(SuiteRunner.java:340)
at org.testng.SuiteRunner.run(SuiteRunner.java:289)
at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
at org.testng.TestNG.runSuitesSequentially(TestNG.java:1293)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1218)
at org.testng.TestNG.runSuites(TestNG.java:1133)
at org.testng.TestNG.run(TestNG.java:1104)
at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:132)
at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeMulti(TestNGDirectoryTestSuite.java:193)
at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:94)
at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:147)
at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:290)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:242)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:121)
Found 1 deadlock.
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #622: Integration tests fixes
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/622#discussion_r110596919
--- Diff: core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestUtils.java ---
@@ -238,7 +238,6 @@ public LocalManagementContext buildUnstarted() {
classLoader);
((RebindManagerImpl) unstarted.getRebindManager()).setPeriodicPersistPeriod(persistPeriod);
unstarted.getRebindManager().setPersister(newPersister, PersistenceExceptionHandlerImpl.builder().build());
- unstarted.getHighAvailabilityManager().disabled();
--- End diff --
Not sure about the implications of removing this. It seems reasonable that we execute the HA code-path in our tests, so I think this is fine. I take it that some of our integration tests assumd that it's enabled?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #622: Integration tests fixes
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/622#discussion_r110596123
--- Diff: core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java ---
@@ -201,6 +227,163 @@ public void testStartsThenChildFailsButWithQuorumCausesAppToStayHealthy() throws
assertUpAndRunningEventually(app);
}
+ /**
+ * Sensor value does not match the order of events. For example events are running, starting,
+ * but sensor value is running.
+ * Causes a problem with ComputeServiceIndicatorsFromChildrenAndMembers which will flag the entity
+ * on-fire event though children are running.
+ */
+ @Test
+ public void testSettingSensorFromThreads() {
+ final TestApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(TestApplication.class));
+ final AttributeSensor<String> TEST_SENSOR = Sensors.newStringSensor("test.sensor");
+
+ final AtomicReference<String> lastSeenState = new AtomicReference<>();
+ app.subscriptions().subscribe(app, TEST_SENSOR, new SensorEventListener<String>() {
+ @Override
+ public void onEvent(SensorEvent<String> event) {
+ lastSeenState.set(event.getValue());
+ log.debug("seen event=" + event);
+ }
+ });
+
+ Task<?> first = mgmt.getExecutionManager().submit(new Runnable() {
+ @Override
+ public void run() {
+ app.sensors().set(TEST_SENSOR, "first");
+ log.debug("set first");
+ }
+ });
+ Task<?> second = mgmt.getExecutionManager().submit(new Runnable() {
+ @Override
+ public void run() {
+ app.sensors().set(TEST_SENSOR, "second");
+ log.debug("set second");
+ }
+ });
+ first.blockUntilEnded();
+ second.blockUntilEnded();
+
+ Asserts.succeedsEventually(new Runnable() {
+ @Override
+ public void run() {
+ EntityAsserts.assertAttributeEquals(app, TEST_SENSOR, lastSeenState.get());
+ }
+ });
+ }
+
+ public static class RecodingChildSensorEnricher extends AbstractMultipleSensorAggregator<Void> {
+ public static AttributeSensor<String> RECORDED_SENSOR = Sensors.newStringSensor("recorded.sensor");
+ List<String> seenValues = new ArrayList<>();
+
+ @Override
+ protected Collection<Sensor<?>> getSourceSensors() {
+ return ImmutableList.<Sensor<?>>of(RECORDED_SENSOR);
+ }
+
+ @Override
+ protected Object compute() {
+ throw new UnsupportedOperationException("Not expected to be called since onUpdated is overriden");
+ }
+
+ @Override
+ protected void setEntityLoadingTargetConfig() {
+ }
+
+ @Override
+ protected void onUpdated() {
+ Iterator<String> values = getValues(RECORDED_SENSOR).values().iterator();
+ if (values.hasNext()) {
+ seenValues.add(values.next());
+ }
+ }
+ }
+
+ /**
+ * Enricher sees state in the wrong order -> running, starting, running
+ *
+ * Indeterministic, fails a couple of times per 100 invocations when run with "mvn test" in the
+ * brooklyn-itest docker container.
+ */
+ @Test(groups="Broken")
+ public void testWrongSensorInitValue() {
+ TestApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(TestApplication.class)
+ .impl(TestApplicationNoEnrichersImpl.class)
+ .enricher(EnricherSpec.create(RecodingChildSensorEnricher.class))
+ .child(EntitySpec.create(TestEntity.class)
+ .impl(TestEntityNoEnrichersImpl.class)));
+
+ Entity child = Iterables.get(app.getChildren(), 0);
+ child.sensors().set(RecodingChildSensorEnricher.RECORDED_SENSOR, "first");
+ child.sensors().set(RecodingChildSensorEnricher.RECORDED_SENSOR, "second");
+
+ final RecodingChildSensorEnricher enricher = getFirstEnricher(app, RecodingChildSensorEnricher.class);
+
+ // setEntity -> onUpdate
+ // CHILD_ADDED -> onUpdate
+ // set RECORDED_SENSOR=first -> onUpdate
+ // set RECORDED_SENSOR=second -> onUpdate
+ Asserts.eventually(Suppliers.ofInstance(enricher.seenValues), CollectionFunctionals.sizeEquals(4));
+
+ boolean isOrdered = Ordering.explicit(MutableList.of("first", "second"))
+ .nullsFirst()
+ .isOrdered(enricher.seenValues);
+ assertTrue(isOrdered, "Unexpected ordering for " + enricher.seenValues);
+ }
+
+ public static class EmittingEnricher extends AbstractEnricher {
+ @Override
+ public void setEntity(@SuppressWarnings("deprecation") org.apache.brooklyn.api.entity.EntityLocal entity) {
+ super.setEntity(entity);
+ this.suppressDuplicates = true;
+ }
+
+ @Override
+ public <T> void emit(Sensor<T> sensor, Object val) {
+ super.emit(sensor, val);
+ }
+ }
+
+ /**
+ * The deduplication logic in AbstractEnricher does not work for parallel invocations.
--- End diff --
javadoc no longer correct, now that you've fixed it. How about:
```
* The deduplication logic in AbstractEnricher previously did not work for parallel invocations.
* It used to do a get and then a compare, so another thread could change the value between
* those two operations.
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #622: Integration tests fixes
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/622
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #622: Integration tests fixes
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/622#discussion_r110593557
--- Diff: core/src/test/java/org/apache/brooklyn/core/entity/ApplicationLifecycleStateTest.java ---
@@ -201,6 +227,163 @@ public void testStartsThenChildFailsButWithQuorumCausesAppToStayHealthy() throws
assertUpAndRunningEventually(app);
}
+ /**
+ * Sensor value does not match the order of events. For example events are running, starting,
--- End diff --
Maybe reword the javadoc - I think this refers to the situation before you made the fix.
How about:
```
/**
* Tests concurrent modifications to a sensor, asserting that the last notification the subscribers
* receives equals the last value that sensor has.
*
* Prior to this being fixed (see https://github.com/apache/brooklyn-server/pull/622), it caused
* problems in ComputeServiceIndicatorsFromChildrenAndMembers: it saw a child transition
* from "running" to "starting", and thus emitted the on-fire event for the parent entity. As asserted
* by this test, the enricher should now always receive the events in the correct order (e.g. "starting",
* "running").
*/
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #622: Integration tests fixes
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/622#discussion_r110603378
--- Diff: core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/RebindTestUtils.java ---
@@ -238,7 +238,6 @@ public LocalManagementContext buildUnstarted() {
classLoader);
((RebindManagerImpl) unstarted.getRebindManager()).setPeriodicPersistPeriod(persistPeriod);
unstarted.getRebindManager().setPersister(newPersister, PersistenceExceptionHandlerImpl.builder().build());
- unstarted.getHighAvailabilityManager().disabled();
--- End diff --
I added this recently in https://github.com/apache/brooklyn-server/pull/607, but no longer needed after the changes in https://github.com/apache/brooklyn-server/pull/617 which make the HA state configurable.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #622: Integration tests fixes
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/622
Thanks for the analysis @aledsage. I split the changes causing the failure in a separate PR (https://github.com/apache/brooklyn-server/pull/641) so we can merge this one independently.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #622: Integration tests fixes
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/622
Thanks @aledsage. Updated the javadocs. Waiting for the new build to finish and will merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---