Duplicate job scheduling in org.drools.time.impl.DefaultTimerJobInstance.call()
-------------------------------------------------------------------------------
Key: JBRULES-3284
URL:
https://issues.jboss.org/browse/JBRULES-3284
Project: Drools
Issue Type: Bug
Security Level: Public (Everyone can see)
Components: drools-core (fusion)
Affects Versions: 5.3.0.Final
Reporter: Richard Calmbach
Assignee: Mark Proctor
See the forum reference above for context. As requested, this is the bug report for issue
F1 in the post. The intro and description are copied from the original post:
I am making extensive use of the event processing features of the Drools rule engine.
Upgrading from Drools 5.2.0.Final to Drools 5.3.0.Final broke 47 of my unit tests and also
broke my functional tests. There seem to be multiple changes in Drools 5.3.0 that cause
incorrect behavior and/or break backward compatibility. Here are the results of my
investigation so far.
Issue F1:
While tracking down the failures of my unit tests, I experimented with the Broker example,
and while probably not finding the root cause of the broken unit tests, I nonetheless came
across what clearly seems to be incorrect behavior in the DefaultTimerJobInstance.call()
method. The bug only reveals itself after all input has been processed, so it is masked by
the fact that running the Broker demo through the entire sequence in stocktickstream.dat
(1100 lines) takes a long time. In order to reveal the problem more quickly, run the demo
with only the first 14 lines in stocktickstream.dat. Do so for both the 5.2.0 Broker demo
(against Drools 5.2.0) and the 5.3.0 Broker demo (against Drools 5.3.0). The Broker
example code in both versions is identical, so only the Drools-internal code changes
matter.
When running the 5.2.0 Broker demo to the end, you get one java.text.ParseException (given
the structure of the example code, that's expected, albeit not elegant, and not the
focus of our investigation). In particular, no matter how many lines stocktickstream.dat
contains, you always get exactly one ParseException at the end.
Contrast this with running the 5.3.0 Broker demo: At the end you get N occurrences of
java.text.ParseException, where N is the number of lines in stocktickstream.dat. So for 14
lines you get 14 occurrences of ParseException. Looking at two specific methods shows us
why:
Method org.drools.examples.broker.events.EventFeeder.FeedJob.execute(JobContext):
{code}
public void execute(JobContext context) {
this.sink.receive( ((FeedContext) context).event );
if ( this.source.hasNext() ) {
((FeedContext) context).setEvent( this.source.getNext() );
this.trigger.setNextFireTime( ((FeedContext) context).getEvent().getDate()
);
clock.scheduleJob( this,
context,
trigger );
}
}
{code}
Note in particular how this method already takes care of scheduling the next job execution
by updating the next fire time of the job's existing FeedTrigger instance.
Unfortunately, in Drools 5.3.0, DefaultTimerJobInstance.call() does a duplicate scheduling
of the same job:
Method org.drools.time.impl.DefaultTimerJobInstance.call():
{code}
public Void call() throws Exception {
this.trigger.nextFireTime(); // need to pop
if ( handle.isCancel() ) {
return null;
}
this.job.execute( this.ctx );
if ( handle.isCancel() ) {
return null;
}
// our triggers allow for flexible rescheduling
Date date = this.trigger.hasNextFireTime();
if ( date != null ) {
scheduler.internalSchedule( this );
}
return null;
}
{code}
So, every job is duplicated and that's why there are 2*N calls to
org.drools.examples.broker.events.StockTickPersister.load() instead of N.
I think the rescheduling inside DefaultTimerJobInstance.call() is incorrect. For one, it
breaks backward compatibility, and it is unexpected. The job should be in charge of
deciding whether there is another job to schedule or what to do. Implicitly scheduling the
next job just by updating the trigger time is a little too much magic.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.jboss.org/secure/ContactAdministrators!default.jspa
For more information on JIRA, see:
http://www.atlassian.com/software/jira