You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by GitBox <gi...@apache.org> on 2020/11/19 19:07:29 UTC
[GitHub] [samza] lakshmi-manasa-g opened a new pull request #1445: [WIP-tests tbd] SAMZA-2604: Datamodel change to capture physical container id for AM HA
lakshmi-manasa-g opened a new pull request #1445:
URL: https://github.com/apache/samza/pull/1445
**Feature:** Main feature is Cluster based Job coordinator (aka AM) high availability (HA) (TODO: sep/doc how?). The feature ensures that the new AM can establish connection with already running containers to avoid restarting all running containers when AM dies. This PR enables capturing of the physical execution environment container id (ex: yarn container id "container_123_123") mapping to Samza logical processor id (ex: "0"). In future PRs, this mapping will be used by the new AM.
**Changes:**
1. Introduce new Coordinator Stream Message and manager to read/write this message
2. Container upon launch will write to c-stream, its logical and physical id
3. Job Coordinator(AM) upon launch will read the mapping of all containers from c-stream.
**Tests:**
1. added unit test for new manager
2. working on tests for other classes as they have no coverage for relevant code.
**API changes:**
1. New c-stream message
**Usage instructions:** None
**Upgrade instructions:** Backwards compatible. N/A
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] mynameborat merged pull request #1445: SAMZA-2604: Datamodel change to capture physical container id for AM HA
Posted by GitBox <gi...@apache.org>.
mynameborat merged pull request #1445:
URL: https://github.com/apache/samza/pull/1445
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] mynameborat commented on pull request #1445: SAMZA-2604: Datamodel change to capture physical container id for AM HA
Posted by GitBox <gi...@apache.org>.
mynameborat commented on pull request #1445:
URL: https://github.com/apache/samza/pull/1445#issuecomment-736207408
Please resolve the conflict so that I can merge the PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] mynameborat commented on a change in pull request #1445: SAMZA-2604: Datamodel change to capture physical container id for AM HA
Posted by GitBox <gi...@apache.org>.
mynameborat commented on a change in pull request #1445:
URL: https://github.com/apache/samza/pull/1445#discussion_r532992365
##########
File path: samza-core/src/main/java/org/apache/samza/runtime/ContainerLaunchUtil.java
##########
@@ -151,10 +151,9 @@ private static void run(
if (new JobConfig(config).getJobCoordinatorHighAvailabilityEnabled()) {
ExecutionContainerIdManager executionContainerIdManager = new ExecutionContainerIdManager(
- new NamespaceAwareCoordinatorStreamStore(coordinatorStreamStore, SetExecutionContainerIdMapping.TYPE));
- if (executionContainerIdManager != null && execEnvContainerId.isPresent()) {
- executionContainerIdManager.writeExecutionEnvironmentContainerIdMapping(containerId, execEnvContainerId.get());
- }
+ new NamespaceAwareCoordinatorStreamStore(coordinatorStreamStore, SetExecutionEnvContainerIdMapping.TYPE));
Review comment:
Can go inside the `ifPresent` block so that we don't create it unless it is available? Why this is optional by the way? Seems like this is required and we should bail out. unless its used in local flow where it isn't populated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org