[
https://issues.apache.org/jira/browse/TEZ-2581?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14976371#comment-14976371
]
Bikas Saha commented on TEZ-2581:
---------------------------------
tez-dag/src/main/java/org/apache/tez/dag/app/TaskCommunicatorManager.java
Typo - fore recovery
In the patch we are sending completion events to TA directly. Is the ordering
of DM events and completion events via serializing through the Vertex not
needed anymore? Does the same ordering apply to the event being sent to the TA
-> DM events must be sent before completion events? If yes, then can the code
have some comments for that. A test that verifies the ordering constraint would
be even better.
Create a common method to filter taGeneratedEvents/
tez-dag/src/main/java/org/apache/tez/dag/app/dag/event/DAGEventRecoverEvent.java
Why is desire state still needed
tez-dag/src/main/java/org/apache/tez/dag/app/dag/event/VertexEventRecoverVertex.java
Why is this still needed
tez-dag/src/main/java/org/apache/tez/dag/history/events/VertexInitGeneratedEvent.java
&
tez-dag/src/main/java/org/apache/tez/dag/history/events/TaskAttemptGeneratedEvent.java
Event serde code should be in Event or some common place (e.g. ProtoConverters)
- not in RecoverConverters where others will not try to search for.
IMO, event constructors should be dumb and not have filters for what events are
relevant. That is the reponsibility of the code that saves/loads/uses this data.
{code} if (EnumSet.of(EventType.DATA_MOVEMENT_EVENT,
EventType.COMPOSITE_DATA_MOVEMENT_EVENT,
EventType.ROOT_INPUT_INITIALIZER_EVENT,
EventType.VERTEX_MANAGER_EVENT)
.contains(event.getEventType())) {
this.events.add(event);
}{code}
tez-dag/src/main/java/org/apache/tez/dag/history/logging/impl/HistoryEventJsonConversion.java
Remove dead/commented code - // case
VERTEX_DATA_MOVEMENT_EVENTS_GENERATED: and others
Unrelated removal?
{code} otherInfo.put(ATSConstants.START_TIME, event.getStartTime());
- otherInfo.put(ATSConstants.SCHEDULED_TIME, event.getScheduledTime());
{code}
NUM_TASKS in eventInfo and also in otherInfo?
Missing inputSpec, locations - its ok to skip them if they are not relevant but
please put comments saying its done on purpose
> Umbrella for Tez Recovery Redesign
> ----------------------------------
>
> Key: TEZ-2581
> URL: https://issues.apache.org/jira/browse/TEZ-2581
> Project: Apache Tez
> Issue Type: Improvement
> Reporter: Jeff Zhang
> Assignee: Jeff Zhang
> Attachments: TEZ-2581-WIP-1.patch, TEZ-2581-WIP-2.patch,
> TEZ-2581-WIP-3.patch, TEZ-2581-WIP-4.patch, TEZ-2581-WIP-5.patch,
> TEZ-2581-WIP-6.patch, TezRecoveryRedesignProposal.pdf,
> TezRecoveryRedesignV1.1.pdf
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
