You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by pavan kumar kolamuri <pa...@gmail.com> on 2016/01/05 12:52:50 UTC
Review Request 41926: Retry does not update the state of the instance
in the database
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/
-----------------------------------------------------------
Review request for Falcon.
Bugs: FALCON-1719
https://issues.apache.org/jira/browse/FALCON-1719
Repository: falcon-git
Description
-------
Retry is not updating the state of the instance in the databases
Diffs
-----
rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
Diff: https://reviews.apache.org/r/41926/diff/
Testing
-------
Thanks,
pavan kumar kolamuri
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by pavan kumar kolamuri <pa...@gmail.com>.
> On Jan. 5, 2016, 12:05 p.m., Pallavi Rao wrote:
> > rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java, line 63
> > <https://reviews.apache.org/r/41926/diff/1/?file=1181883#file1181883line63>
> >
> > It is better to knock this method off and ensure we get the workflow engine based on the entity all the time.
> >
> > Inside the method you can use WorkflowEngineFactory.getWorkflowEngine() for default/backward compatibility.
Make sense will update
- pavan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/#review112825
-----------------------------------------------------------
On Jan. 5, 2016, 11:52 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41926/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 11:52 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1719
> https://issues.apache.org/jira/browse/FALCON-1719
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Retry is not updating the state of the instance in the databases
>
>
> Diffs
> -----
>
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
> rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
>
> Diff: https://reviews.apache.org/r/41926/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/#review112825
-----------------------------------------------------------
rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java (line 63)
<https://reviews.apache.org/r/41926/#comment173313>
It is better to knock this method off and ensure we get the workflow engine based on the entity all the time.
Inside the method you can use WorkflowEngineFactory.getWorkflowEngine() for default/backward compatibility.
- Pallavi Rao
On Jan. 5, 2016, 11:52 a.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41926/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 11:52 a.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1719
> https://issues.apache.org/jira/browse/FALCON-1719
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Retry is not updating the state of the instance in the databases
>
>
> Diffs
> -----
>
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
> rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
>
> Diff: https://reviews.apache.org/r/41926/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/#review112841
-----------------------------------------------------------
Ship it!
Ship It!
- Pallavi Rao
On Jan. 5, 2016, 12:42 p.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41926/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 12:42 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1719
> https://issues.apache.org/jira/browse/FALCON-1719
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Retry is not updating the state of the instance in the databases
>
>
> Diffs
> -----
>
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
> rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
>
> Diff: https://reviews.apache.org/r/41926/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by pavan kumar kolamuri <pa...@gmail.com>.
> On Jan. 5, 2016, 10:09 p.m., Balu Vellanki wrote:
> > rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java, line 72
> > <https://reviews.apache.org/r/41926/diff/2/?file=1181893#file1181893line72>
> >
> > If entity is defined, the wfEngine should be the workflow endpoint defined in the cluster of this entity. If for any reason getting this wfEngine fails, shouldnt we propagate the error up to the user? Please help me understand why we ignore the exception here...
Previously we always used to return the wfengine based on property defined in startup.properties (workflow.engine.impl). With Native Scheduler even though this property is oozieworkflow engine, there is chance that process is scheduled using native scheduler, in that case we should return workflow engine based on what workflow engine it was scheduled. If it fails while getting wfengine based on entity it should return default workflow engine defined in props which is happening previously , thats why for backward compatibility we should always return default workflow engine.
- pavan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/#review112939
-----------------------------------------------------------
On Jan. 5, 2016, 12:42 p.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41926/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 12:42 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1719
> https://issues.apache.org/jira/browse/FALCON-1719
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Retry is not updating the state of the instance in the databases
>
>
> Diffs
> -----
>
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
> rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
>
> Diff: https://reviews.apache.org/r/41926/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
> On Jan. 5, 2016, 10:09 p.m., Balu Vellanki wrote:
> > rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java, line 72
> > <https://reviews.apache.org/r/41926/diff/2/?file=1181893#file1181893line72>
> >
> > If entity is defined, the wfEngine should be the workflow endpoint defined in the cluster of this entity. If for any reason getting this wfEngine fails, shouldnt we propagate the error up to the user? Please help me understand why we ignore the exception here...
>
> pavan kumar kolamuri wrote:
> Previously we always used to return the wfengine based on property defined in startup.properties (workflow.engine.impl). With Native Scheduler even though this property is oozieworkflow engine, there is chance that process is scheduled using native scheduler, in that case we should return workflow engine based on what workflow engine it was scheduled. If it fails while getting wfengine based on entity it should return default workflow engine defined in props which is happening previously , thats why for backward compatibility we should always return default workflow engine.
I don't fully understand the backward compatibility part here. Intention here is going forward entities should be scheduled using Native scheduler even if user doesn't pass the scheduler option in the cmd line? And if there was any error instanting the Native scheduler class then fall back to default wf engine?
If so shouldn't this be done in WorkflowEngineFactory.getWorkflowEngine(entity) and not only here.
Also lets say this particular entity was scheduled the first time using native scheduler and it failed. Now for rerun if getting wfengine fails and default wf engine is returned wouldn't job execution fail?
- Sowmya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/#review112939
-----------------------------------------------------------
On Jan. 5, 2016, 12:42 p.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41926/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 12:42 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1719
> https://issues.apache.org/jira/browse/FALCON-1719
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Retry is not updating the state of the instance in the databases
>
>
> Diffs
> -----
>
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
> rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
>
> Diff: https://reviews.apache.org/r/41926/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by Ajay Yadava <aj...@gmail.com>.
> On Jan. 5, 2016, 10:09 p.m., Balu Vellanki wrote:
> > rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java, line 72
> > <https://reviews.apache.org/r/41926/diff/2/?file=1181893#file1181893line72>
> >
> > If entity is defined, the wfEngine should be the workflow endpoint defined in the cluster of this entity. If for any reason getting this wfEngine fails, shouldnt we propagate the error up to the user? Please help me understand why we ignore the exception here...
>
> pavan kumar kolamuri wrote:
> Previously we always used to return the wfengine based on property defined in startup.properties (workflow.engine.impl). With Native Scheduler even though this property is oozieworkflow engine, there is chance that process is scheduled using native scheduler, in that case we should return workflow engine based on what workflow engine it was scheduled. If it fails while getting wfengine based on entity it should return default workflow engine defined in props which is happening previously , thats why for backward compatibility we should always return default workflow engine.
>
> Sowmya Ramesh wrote:
> I don't fully understand the backward compatibility part here. Intention here is going forward entities should be scheduled using Native scheduler even if user doesn't pass the scheduler option in the cmd line? And if there was any error instanting the Native scheduler class then fall back to default wf engine?
>
> If so shouldn't this be done in WorkflowEngineFactory.getWorkflowEngine(entity) and not only here.
>
>
> Also lets say this particular entity was scheduled the first time using native scheduler and it failed. Now for rerun if getting wfengine fails and default wf engine is returned wouldn't job execution fail?
>
> pavan kumar kolamuri wrote:
> Also lets say this particular entity was scheduled the first time using native scheduler and it failed. Now for rerun if getting wfengine fails and default wf engine is returned wouldn't job execution fail?
> Yes you are right let me explain all cases ,
>
> WorkflowEngineFactory.getWorkflowEngine(Entity entity) throw FalconException in three cases
> 1) When it didnt find org.apache.falcon.workflow.engine.FalconWorkflowEngine this class in class path
> 2) When StateStore is not initialized
> 3) FalconException will be thrown if entity is not present in configStore.
>
> I thought when org.apache.falcon.workflow.engine.FalconWorkflowEngine doesn't exists its better to return defaultworkflowengine but looks like it might not occur since FalconWorkflowEngine will be present always from now .
> Second case is not possible it won't throw FalconException it might throw other runtime exceptions
> Third case won't occur generally but still we shouldn't catch exception and return Default WorkflowEngine.
I agree with Sowmya Ramesh & Balu Vellanki here, we shouldn't return default workflow engine if there are any exceptions, we should just let it fail in those cases. It is better to fail then return a default and see some obscure results / exceptions.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/#review112939
-----------------------------------------------------------
On Jan. 5, 2016, 12:42 p.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41926/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 12:42 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1719
> https://issues.apache.org/jira/browse/FALCON-1719
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Retry is not updating the state of the instance in the databases
>
>
> Diffs
> -----
>
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
> rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
>
> Diff: https://reviews.apache.org/r/41926/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by pavan kumar kolamuri <pa...@gmail.com>.
> On Jan. 5, 2016, 10:09 p.m., Balu Vellanki wrote:
> > rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java, line 72
> > <https://reviews.apache.org/r/41926/diff/2/?file=1181893#file1181893line72>
> >
> > If entity is defined, the wfEngine should be the workflow endpoint defined in the cluster of this entity. If for any reason getting this wfEngine fails, shouldnt we propagate the error up to the user? Please help me understand why we ignore the exception here...
>
> pavan kumar kolamuri wrote:
> Previously we always used to return the wfengine based on property defined in startup.properties (workflow.engine.impl). With Native Scheduler even though this property is oozieworkflow engine, there is chance that process is scheduled using native scheduler, in that case we should return workflow engine based on what workflow engine it was scheduled. If it fails while getting wfengine based on entity it should return default workflow engine defined in props which is happening previously , thats why for backward compatibility we should always return default workflow engine.
>
> Sowmya Ramesh wrote:
> I don't fully understand the backward compatibility part here. Intention here is going forward entities should be scheduled using Native scheduler even if user doesn't pass the scheduler option in the cmd line? And if there was any error instanting the Native scheduler class then fall back to default wf engine?
>
> If so shouldn't this be done in WorkflowEngineFactory.getWorkflowEngine(entity) and not only here.
>
>
> Also lets say this particular entity was scheduled the first time using native scheduler and it failed. Now for rerun if getting wfengine fails and default wf engine is returned wouldn't job execution fail?
Also lets say this particular entity was scheduled the first time using native scheduler and it failed. Now for rerun if getting wfengine fails and default wf engine is returned wouldn't job execution fail?
Yes you are right let me explain all cases ,
WorkflowEngineFactory.getWorkflowEngine(Entity entity) throw FalconException in three cases
1) When it didnt find org.apache.falcon.workflow.engine.FalconWorkflowEngine this class in class path
2) When StateStore is not initialized
3) FalconException will be thrown if entity is not present in configStore.
I thought when org.apache.falcon.workflow.engine.FalconWorkflowEngine doesn't exists its better to return defaultworkflowengine but looks like it might not occur since FalconWorkflowEngine will be present always from now .
Second case is not possible it won't throw FalconException it might throw other runtime exceptions
Third case won't occur generally but still we shouldn't catch exception and return Default WorkflowEngine.
- pavan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/#review112939
-----------------------------------------------------------
On Jan. 5, 2016, 12:42 p.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41926/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 12:42 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1719
> https://issues.apache.org/jira/browse/FALCON-1719
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Retry is not updating the state of the instance in the databases
>
>
> Diffs
> -----
>
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
> rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
>
> Diff: https://reviews.apache.org/r/41926/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/#review112939
-----------------------------------------------------------
rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java (line 72)
<https://reviews.apache.org/r/41926/#comment173428>
If entity is defined, the wfEngine should be the workflow endpoint defined in the cluster of this entity. If for any reason getting this wfEngine fails, shouldnt we propagate the error up to the user? Please help me understand why we ignore the exception here...
- Balu Vellanki
On Jan. 5, 2016, 12:42 p.m., pavan kumar kolamuri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41926/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 12:42 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1719
> https://issues.apache.org/jira/browse/FALCON-1719
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Retry is not updating the state of the instance in the databases
>
>
> Diffs
> -----
>
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
> rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
> rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
> rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
>
> Diff: https://reviews.apache.org/r/41926/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pavan kumar kolamuri
>
>
Re: Review Request 41926: Retry does not update the state of the
instance in the database
Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41926/
-----------------------------------------------------------
(Updated Jan. 5, 2016, 12:42 p.m.)
Review request for Falcon.
Changes
-------
Updated Review comments
Bugs: FALCON-1719
https://issues.apache.org/jira/browse/FALCON-1719
Repository: falcon-git
Description
-------
Retry is not updating the state of the instance in the databases
Diffs (updated)
-----
rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java 582cb15
rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java 64c566e
rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java ee31952
rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 64177a4
rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 61aa3e1
Diff: https://reviews.apache.org/r/41926/diff/
Testing
-------
Thanks,
pavan kumar kolamuri