Community
Participate
Working Groups
WireAdmin spec allows flow control over the wire: Section 108.7. There are several filters which depend upon the "previous" value which seems to be defined by WIREVALUE_PREVIOUS as the previous data value reported to the Consumer. That is, the Consumer actually received the value. Most of the filter types are based upon the previous reported value. So when the first produced value is updated onto the wire, there has not been a previous value, so none of those filter types will report the value to the consumer and we are now caught in a loop of never having a previous value and never delivering a value. The CT assumes that an implementation will "prime the pump" by considering the first undelivered value as a previous value for purposes of evaluating the filter for the next value. The OSGi CT methods, testValueFilteringDeltaAbsolute and testValueFilteringDeltaRelative, both fail because no values are ever delivered to the test Consumer.
It would be good to get this in RC4 so Equinox can be used as the RI for WireAdmin. BJ, please attach the patch.
Created attachment 197113 [details] Fix to prime the pump for previous values The patch "primes the pump" by setting the first value passed to the wire as the previous value even if that value is not actually passed to the consumer. The patch also makes the proper distinction between lastValue, which is the last value provided by the Producer, and previousValue, which is the previous value supplied to the Consumer. When wire flow control is in effect, these 2 values are often different since some values are not actually supplied to the consumer. Finally the patch adds finally block to properly set lastValue and/or previousValue due to early return from the method.
John and Stoyan, please review.
Initial review of the patch looks good, but I will admit that I am confused by the field names "lastValue" and "previousValue". perhaps "lastFromProducer" and "lastToConsumer". Need to up date copyright date.
(In reply to comment #4) > Initial review of the patch looks good, but I will admit that I am confused by > the field names "lastValue" and "previousValue". perhaps "lastFromProducer" > and "lastToConsumer". > The names match terms used on the spec. I felt it was important to use matching names. > Need to up date copyright date. Do I just change 2008 to 2011? Or are more changes needed?
(In reply to comment #5) > (In reply to comment #4) > > Initial review of the patch looks good, but I will admit that I am confused by > > the field names "lastValue" and "previousValue". perhaps "lastFromProducer" > > and "lastToConsumer". > > > > The names match terms used on the spec. I felt it was important to use matching > names. Fair enough, I was not reading the spec when I reviewed. > > > Need to up date copyright date. > > Do I just change 2008 to 2011? Or are more changes needed? Yes, that will do. You could add a line under Contributors: IBM Corporation - bug fix 347974
Created attachment 197125 [details] [updated] Fix to prime the pump for previous value Updated patch with modified copyright notice.
Looks good.
The patch is OK and fixes the original bug. But it seems to me it introduces a new bug. It is calling parent.notifyListeners(this, WireAdminEvent.WIRE_TRACE, null) regardless the consumer's updated method might not have been called. This method must be called only if the consumer's updated method is called.
Created attachment 197150 [details] [updated2] Fix to prime the pump for previous value This is a small update of the patch. It fixes the only issue found in the previous patch
(In reply to comment #9) > The patch is OK and fixes the original bug. But it seems to me it introduces a > new bug. > It is calling parent.notifyListeners(this, WireAdminEvent.WIRE_TRACE, null) > regardless the consumer's updated method might not have been called. > This method must be called only if the consumer's updated method is called. It is supposed to trace even if the consumer is not called. From the spec 108.11: WIRE_TRACE - The Producer service has called the Wire.update(Object) method with a new value or the Producer service has returned from the Producer.polled(Wire) method. I had fixed this bug in my patch but had forgotten to mention it in my patch description.
BJ, Stoyan and I discussed this and believe we should go with Stoyan's patch. It appears the javadoc for wireadmin is in conflict. The current RI behaves as Stoyan's patch and we should base on that for the decision. We will get the spec clarified to be consistent in the next spec release.
Fix released to HEAD for 3.7 RC4.
