Skip to content

Conversation

@tillrohrmann
Copy link
Contributor

What is the purpose of the change

Properly fences the ResourceManager by letting it extend the FencedRpcEndpoint.
Moreover, this PR introduces a ResourceManagerId which replaces the UUID as
leader id/fencing token. This will give us more type safety when defining rpc
interfaces.

This PR is based on #4580 and #4578.

Brief change log

  • Let ResourceManager extend from FencedRpcEndpoint
  • Introduce the ResourceManagerId which replaces the ResourceManager's leader id
  • Adapt the ResourceManagerGateway to not require explicitly sending the ResourceManagerId (this is implicitly done by the FencedAkkaInvocationHandler)
  • Change the RetryingRegistration to connect to FencedRpcEndpoints

Verifying this change

This change is already covered by existing tests, such as the FencedRpcEndpointTest.

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 fenceResourceManager branch 5 times, most recently from 1c97bc7 to 7a59896 Compare August 26, 2017 21:20
Properly fences the ResourceManager by letting it extend the FencedRpcEndpoint.
Moreover, this PR introduces a ResourceManagerId which replaces the UUID as
leader id/fencing token. This will give us more type safety when defining rpc
interfaces.
@tillrohrmann
Copy link
Contributor Author

Travis passed. Merging this PR.

@asfgit asfgit closed this in e70de0e Sep 4, 2017
@tillrohrmann tillrohrmann deleted the fenceResourceManager branch September 14, 2017 13:00
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.

2 participants