[shawkins]

Mark,

Yes reset can also throw a DNA and reset should logically have the responsibility of handling any initialization for the next execute.  I'll also update the code to so that a DNA thrown from execute will not cause reset to be called multiple times.

Steve

[markaddleman]

Great.  Filed TEIID-2308

[markaddleman]

Here's my first cut at the state diagram

[rareddy]

Isn't "reset" already means initialization of next execute cycle? A simple return from "reset" goes to "execute", how is throwing DNF from it differs from it? DNF thrown from "reset" will call "reset" again? if not, this exception has different meanings in different calls.

I believe the execution model is getting more complex to understand, if the intension is to restart the next execution cycle in the middle of current execution, at minimum please use different exception like "TerminateExecutionException" to communicate the intent.

Thanks.

Ramesh..

[markaddleman]

[markaddleman]

Crap.  That didn't come out as nicely as I'd hope.  I'll try to produce a better image

[rareddy]

ok, based on the your state diagram this is my interpretation

1) after the first cycle of execution, when "reset" is called, it throws a DNF or simply returns.

2) If "reset" threw the DNF, it already knows from that the source is not ready to provide data for next cycle, where in fact the execution for the data for next cycle has not even started yet. If, say "reset" is start of the new cycle, then it this has knowledge of "execute" call, that seems bad.

3) if, "reset" threw the DNF with time delay, then it is no different than what we have with "execute" having the capability to throw DNF.

I had thought it would like this

[shawkins]

Ramesh,

The client part is not accurate.  It would be best to just show the transition is from close to reset when/if the next processing iteration begins.  Client close/cancel can happen at any point.  Cancel will asynchly call Execution cancel.  Then client cancel/close both cause a transition to close if not currently closed and then the subsequent transition to dispose.

My intention was that a DNA exception is a retry.  To the engine that is really no different if thrown from execute, next, or reset - we should simply loop.  The code currently behaves like that for next/reset anyway.

TEIID-2308 doesn't quite capture what I meant.  I'm going to make a small change so that a DNA thrown from the execute method will not call reset again.  I gather you are questioning wheter a DNA from execute means that execute succeeded, or just that it seems redundant given that you can throw DNA from the next method. The retry paradigm seems consistent and is less code than explicitly disallowing the exception from other states. 

Other than having the handling defined, I would think that uses of a DNA exception from a method other than next is esoteric.

Steve

[rareddy]

Steve,

The client part is not accurate.  It would be best to just show the transition is from close to reset when/if the next processing iteration begins.  Client close/cancel can happen at any point.  Cancel will asynchly call Execution cancel.  Then client cancel/close both cause a transition to close if not currently closed and then the subsequent transition to dispose.

Yeah, I was going to draw with dotted line, that is not right.

My intention was that a DNA exception is a retry.  To the engine that is really no different if thrown from execute, next, or reset - we should simply loop.  The code currently behaves like that for next/reset anyway.

That is better explanation, I can not envision a usecase, but I am sure Mark can fit his usecase with this enhancement. I am still getting hung up on the name DNF and meaning it is bringing. I wish we use BlockedException for that intent.

I gather you are questioning wheter a DNA from execute means that execute succeeded, or just that it seems redundant given that you can throw DNA from the next method.

just that it seems redundant to throw from any method other than "next". I was advocating explicitly not to allow to make flow simple.

[markaddleman]

anyway.

That is better explanation, I can not envision a usecase, but I am sure Mark can fit his usecase with this enhancement. I am still getting hung up on the name DNF and meaning it is bringing. I wish we use BlockedException for that intent.

My preference is that DNA thrown from reset() causes the execution to restart from execute() when dataAvailable() and treat DNA thrown from every else as a loop but, as long as the behavior is well doc'd, I don't think it matters very much.

I agree that these names are poor given what we're trying to do.  In fact, I don't think exceptions are the right tool for conveying the intent (though, I am sympathetic to the history of the feature). 

I'll give one more try to describing our usecase, though I'm not sure I can convey it much differently than what I've done before.It's funny because it's hard for me to see why anyone would want the behavior different than what we've settled on  

Our data sources are something like Linux top:  every once in a while, our "top" data source samples all of the processes running on the system and provides a bunch of data about each one.  The sampling is done on operating system events rather than a timer.  Once the OS event occurs, the source can quickly gather all of the necessary information.  Further, the system architecture requires that the first time a client requests data from the source, it samples.  Mapping this into Teiid's current execution lifecycle:

1. execute()
   1. If run for the first time, prepare to sample
   2. If not the first time and it's not time to sample, throw DNA
   3. If not the first time and it is time to sample, prepare to sample
2. next()
   1. Perform the sampling and return each row
   2. Return null when we've run out of processes
3. close() - Return any OS resources
4. reset() - Gather the new execution from Teiid
5. dispose() -Nothing

Mapping this into my preferred execution lifecycle (where DNA from reset goes to execute):

1. execute() - Prepare to sample
2. next()
   1. Perform the sampling and return each row
   2. Return null when we've run out of processes
3. close() - Return any OS resources
4. reset()
   1. Gather the new execution from Teiid
   2. Throw DNA
5. displose() - Nothing

Now that I write this out, I realize there is a possible timing issue that occurs when dataAvailable() is called some time between execute() and reset().  My preference is that dataAvailable() always refers to the next execution.  Thus, if dataAvailable() was called asynchronously some time before throwing DNA, then the execution would restart immediately.

Does this usecase make any more sense?

[shawkins]

https://issues.jboss.org/browse/TEIID-2308 has been resolved.  reset/execute/next will now all just be retried if a DNA exeception is thrown.

> Now that I write this out, I realize there is a possible timing issue that occurs when dataAvailable() is called some time between execute() and reset().  My preference is that dataAvailable() always refers to the next execution.  Thus, if dataAvailable() was called asynchronously some time before throwing DNA, then the execution would restart immediately.

See the ExecutionContextImpl.isDataAvailable(), whenever the engine polls for data availablity - which is checked before blocking and if no delay/time is specified - the flag is cleared.  As long as the DNA is not strict the plan will effectively continue to process if dataAvailable has been called.  This approach does have a side effect that an older DNA event may be consumed later than anticipated, since it is not cleared for every next call.  I could clean this up a little to consume the flag more proactively, but currently at the worst case non-strict DNA will try to resume to quickly, which should just result in another DNA.

> where DNA from reset goes to execute

I think you're mapping still makes sense if DNA is thrown from reset and we retry reset.  Essentially you are using it as a guard to execute and only allow it to proceed when your result is ready.  Whether that guard logic is in execute or reset seems to just be about keeping the reusable/asynch concerns out of the basic translator logic.  In that regard using reset makes more sense.

Steve

[shawkins]

Just to follow up on an earilier point, with https://issues.jboss.org/browse/TEIID-2316 access nodes will release partial batches before blocking regardless of the scenario.