Skip to content

Conversation

@tillrohrmann
Copy link
Contributor

What is the purpose of the change

This commit implements the CheckpointConfigHandler which now returns a
CheckpointConfigInfo object if checkpointing is enabled. In case that
checkpointing is disabled for a job, it will return a 404 response.

Verifying this change

This change added tests and can be verified as follows:

  • CheckpointConfigInfoTest checks the marshalling of CheckpointConfigInfo

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)

@tillrohrmann tillrohrmann force-pushed the portCheckpointConfigHandler branch from 76ff833 to e77fb7a Compare September 29, 2017 08:37
Copy link
Contributor

@zentol zentol left a comment

Choose a reason for hiding this comment

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

+1 with a minor comment

public static final String FIELD_NAME_CHECKPOINT_MAX_CONCURRENT = "max_concurrent";

public static final String FIELD_NAME_EXTERNALIZED_CHECKPOINT_CONFIG = "externalization";

Copy link
Contributor

Choose a reason for hiding this comment

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

remove empty line (should fail checkstyle)

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 catching it. Will fix it.

…ndlers

The ExecutionGraphCache replaces the ExecutionGraphHolder. Unlike the latter, the former
does not expect the AccessExecutionGraph to be the true ExecutionGraph. Instead it assumes
it to be the ArchivedExecutionGraph. Therefore, it invalidates the cache entries after
a given time to live period. This will trigger requesting the AccessExecutionGraph again
and, thus, updating the ExecutionGraph information for the ExecutionGraph based REST
handlers.

In order to avoid memory leaks, the WebRuntimeMonitor starts now a periodic cleanup task
which triggers ExecutionGraphCache.cleanup. This methods releases all cache entries which
have exceeded their time to live. Currently it is set to 20 * refreshInterval of the
web gui.

This closes apache#4728.
This commit implements the CheckpointConfigHandler which now returns a
CheckpointConfigInfo object if checkpointing is enabled. In case that
checkpointing is disabled for a job, it will return a 404 response.
@tillrohrmann tillrohrmann force-pushed the portCheckpointConfigHandler branch from e77fb7a to 4259fcc Compare October 2, 2017 12:25
@tillrohrmann
Copy link
Contributor Author

Thanks for the review @zentol. Merging this PR once Travis gives green light.

@asfgit asfgit closed this in b41f5a6 Oct 2, 2017
@tillrohrmann tillrohrmann deleted the portCheckpointConfigHandler branch October 2, 2017 20:32
yew1eb pushed a commit to yew1eb/flink that referenced this pull request Oct 4, 2017
This commit implements the CheckpointConfigHandler which now returns a
CheckpointConfigInfo object if checkpointing is enabled. In case that
checkpointing is disabled for a job, it will return a 404 response.

This closes apache#4744.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants