[ 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)