| Summary: | [Discovery] [ZooDiscovery] GUID contains characters to confuse Zookeeper nodes buildup | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [RT] ECF | Reporter: | Wim Jongman <wim.jongman> | ||||
| Component: | ecf.discovery | Assignee: | Wim Jongman <wim.jongman> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | ahmed.aadel, bugs.eclipse.org, slewis | ||||
| Version: | unspecified | ||||||
| Target Milestone: | 3.3.0 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows Vista | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Wim Jongman
Hi Wim. If the patch can be applied in next couple of days and tested and reviewed by a couple of committers before next Saturday 5/5, we should be able to get into RC4+1 (Monday, 5/7). Setting target milestone to 3.3.0. I hope we can get this in for RC4+1. Created attachment 171092 [details]
Patch - See description
Here is a patch that fixes it along with other instabilities with the failing test on ecf2. I have setup a stresstest where every 15 seconds a service is activated by DS, picked up by Zookeeper and delivered to consumers. The host and three consumers are running already for three days non-stop and they work as expected.
From the stack trace, the following method was behind the failing hudson-zoodiscovery test
org.eclipse.ecf.provider.zookeeper.core.ZooDiscoveryContainer.getServices(ZooDiscoveryContainer.java:384)
Ahmed changed it so that it never returns a null but an empty collection instead even if the watchManager is null.
Ahmed also changed method NodeReader.dispose() because it has the potential of throwing a null pointer exception.
Some unnecessary/error prone threads/ static fields were taken care of.
(In reply to comment #3) > Created an attachment (id=171092) [details] > Patch - See description > > Here is a patch that fixes it along with other instabilities with the failing > test on ecf2. I have setup a stresstest where every 15 seconds a service is > activated by DS, picked up by Zookeeper and delivered to consumers. The host > and three consumers are running already for three days non-stop and they work > as expected. > > From the stack trace, the following method was behind the failing > hudson-zoodiscovery test > org.eclipse.ecf.provider.zookeeper.core.ZooDiscoveryContainer.getServices(ZooDiscoveryContainer.java:384) > > Ahmed changed it so that it never returns a null but an empty collection > instead even if the watchManager is null. > Ahmed also changed method NodeReader.dispose() because it has the potential of > throwing a null pointer exception. > Some unnecessary/error prone threads/ static fields were taken care of. Thanks Wim. I would like to get this patch reviewed, approved, and released to head by end of day tomorrow (Sat 6/5), for RC4+1 build (Sunday evening pacific time). I will try to review this later today/Friday and give my +1. I would like Markus to review as well...and we have Wim's review of course. Hi,
+1
just one issue:
- org.eclipse.ecf.provider.zookeeper.DiscoveryActivator.start(...).new ServiceTracker() {...}.removedService(ServiceReference, Object) has to call super.removedService(...) in org.osgi.util.tracker.ServiceTracker.removedService(ServiceReference, Object)
And please for patches this close to a release only change what is strictly necessary to fix the bug (especially formatting). Makes it a lot easier for us reviewer. :-)
Hi, Thanks for the review. And sorry for the work, you know how it goes... I have added the super call. +1 on patch 171092. Thanks for the quick work and testing. Please release to HEAD and we'll introduce for rc4+1 build 5/6. released to HEAD |