Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-7667] [flip6] Use ArchivedExecutionGraph as serializable AccessExecutionGraph #4727

Merged
merged 1 commit into from Sep 29, 2017

Conversation

tillrohrmann
Copy link
Contributor

What is the purpose of the change

This commit removes AccessExecutionGraph#getCheckpointCoordinator and changes the
AccessExecutionGraph#getJobCheckpointSettings into #getJobCheckpointConfiguration.
The JobCheckpointConfiguration only contains the CheckpointCoordinator relevant
configuration settings and excludes the serialized state backend and the
serialized master hooks. That way we don't send unnecessary information when
the ArchivedExecutionGraph is requested.

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

import java.io.Serializable;
import java.util.Objects;

public class JobCheckpointingConfiguration implements Serializable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing javadoc. Can we also find a better name, since "Settings" and "Configuration" are virtually interchangeable? (Like CHeckpointCoordinatorConfig?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. I will add it and think about a better name.

@tillrohrmann tillrohrmann force-pushed the serializableAccessExecutionGraph branch 2 times, most recently from 81d51c8 to 325105e Compare September 27, 2017 11:34
@tillrohrmann
Copy link
Contributor Author

I've addressed your comments @zentol. The PR should now build and I renamed JobCheckpointingConfiguration into CheckpointCoordinatorConfiguration.

@zentol
Copy link
Contributor

zentol commented Sep 27, 2017

+1.

@tillrohrmann
Copy link
Contributor Author

Thanks for the review @zentol. Rebasing this commit and waiting on Travis.

tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Sep 27, 2017
…sExecutionGraph

This commit removes AccessExecutionGraph#getCheckpointCoordinator and changes the
AccessExecutionGraph#getJobCheckpointSettings into #getJobCheckpointConfiguration.
The JobCheckpointConfiguration only contains the CheckpointCoordinator relevant
configuration settings and excludes the serialized state backend and the
serialized master hooks. That way we don't send unnecessary information when
the ArchivedExecutionGraph is requested.

This closes apache#4727.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Sep 27, 2017
…sExecutionGraph

This commit removes AccessExecutionGraph#getCheckpointCoordinator and changes the
AccessExecutionGraph#getJobCheckpointSettings into #getJobCheckpointConfiguration.
The JobCheckpointConfiguration only contains the CheckpointCoordinator relevant
configuration settings and excludes the serialized state backend and the
serialized master hooks. That way we don't send unnecessary information when
the ArchivedExecutionGraph is requested.

This closes apache#4727.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Sep 27, 2017
…sExecutionGraph

This commit removes AccessExecutionGraph#getCheckpointCoordinator and changes the
AccessExecutionGraph#getJobCheckpointSettings into #getJobCheckpointConfiguration.
The JobCheckpointConfiguration only contains the CheckpointCoordinator relevant
configuration settings and excludes the serialized state backend and the
serialized master hooks. That way we don't send unnecessary information when
the ArchivedExecutionGraph is requested.

This closes apache#4727.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Sep 27, 2017
…sExecutionGraph

This commit removes AccessExecutionGraph#getCheckpointCoordinator and changes the
AccessExecutionGraph#getJobCheckpointSettings into #getJobCheckpointConfiguration.
The JobCheckpointConfiguration only contains the CheckpointCoordinator relevant
configuration settings and excludes the serialized state backend and the
serialized master hooks. That way we don't send unnecessary information when
the ArchivedExecutionGraph is requested.

This closes apache#4727.
…sExecutionGraph

This commit removes AccessExecutionGraph#getCheckpointCoordinator and changes the
AccessExecutionGraph#getJobCheckpointSettings into #getJobCheckpointConfiguration.
The JobCheckpointConfiguration only contains the CheckpointCoordinator relevant
configuration settings and excludes the serialized state backend and the
serialized master hooks. That way we don't send unnecessary information when
the ArchivedExecutionGraph is requested.

This closes apache#4727.
@tillrohrmann
Copy link
Contributor Author

Merging this PR.

@asfgit asfgit merged commit 2dd557f into apache:master Sep 29, 2017
yew1eb pushed a commit to yew1eb/flink that referenced this pull request Oct 4, 2017
…sExecutionGraph

This commit removes AccessExecutionGraph#getCheckpointCoordinator and changes the
AccessExecutionGraph#getJobCheckpointSettings into #getJobCheckpointConfiguration.
The JobCheckpointConfiguration only contains the CheckpointCoordinator relevant
configuration settings and excludes the serialized state backend and the
serialized master hooks. That way we don't send unnecessary information when
the ArchivedExecutionGraph is requested.

This closes apache#4727.
@tillrohrmann tillrohrmann deleted the serializableAccessExecutionGraph branch October 10, 2017 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants