[ 
https://issues.apache.org/jira/browse/TEZ-1384?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14127339#comment-14127339
 ] 

Hitesh Shah commented on TEZ-1384:
----------------------------------

Comments:
   - storeHistoryEvent vs storeCriticalHistoryEvent - does it make sense to 
have a single private function which accepts a boolean param? i.e 
storeHistoryEvent calls storeHIstoryEventInternal(critical= false) and 
storeCriticalHistoryEvent calls storeHIstoryEventInternal(critical = false)  
  - new DAGRecoveryService(this, DAGState.NEW, ... - I see all instances 
accepting a state value which is set to NEW. Maybe remove state as a parameter 
from the ctor of DAG/Vertex/Task/TARecoveryService if the default value is NEW? 
  - use TezUncheckedException instead of RuntimeException 
  - VertexState recover(Object recoverEvent) - why an Object and not the event 
base class? 
  - Does this re-factor help with writing better unit tests ? 

> Move recovery related code into inner class
> -------------------------------------------
>
>                 Key: TEZ-1384
>                 URL: https://issues.apache.org/jira/browse/TEZ-1384
>             Project: Apache Tez
>          Issue Type: Sub-task
>            Reporter: Jeff Zhang
>            Assignee: Jeff Zhang
>         Attachments: Tez-1384-2.patch, Tez-1384.patch
>
>
> Currently each entity (DAG, Vertex, Task, TaskAttempt) has some common 
> recovery code like log history event and restore from history event. These 
> are 2 opposite aspects of recovery. One for store status while the other is 
> for restore status. This jira is for putting these pieces of code together ( 
> in an inner class ). In this way, it is easy to maintain and cut down the 
> possibility that one field is not stored or restored.  



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to