Remove unnecessary services for worker

[hougangliu]

For pytorchJob, MASTER_PORT and MASTER_ADDR are necessary. andMASTER_PORT will be occupied only in Master Pod.

In master Pod, it will

1. Creates sockets for all workers.
2. Waits for all workers to connect.
3. Sends them information about the location of the other processes.

all worker Pods will

1. creates a socket to MASTER_ADDR (master service name): MASTER_PORT
2. send their own location information (ip:randomPort).
3. Receives information about the other workers.
4. Opens a socket and handshakes with all other workers.

That is, MASTER_PORT on worker will not be used. Services for all workers is unnecessary.

[hougangliu]

/cc @johnugeorge

[hougangliu]

/test kubeflow-pytorch-operator-presubmit

[hougangliu]

/test kubeflow-pytorch-operator-presubmit

[coveralls]

Coverage increased (+14.2%) to 85.281% when pulling ee7ba2c on hougangliu:remove-dup-service into 6c75b0c on kubeflow:master.

[hougangliu]

/test kubeflow-pytorch-operator-presubmit

[gaocegege]

/assign @johnugeorge

Ref https://pytorch.org/docs/stable/distributed.html#environment-variable-initialization

[gaocegege]

If the worker does not expose port, how can they Opens a socket and handshakes with all other workers.

[hougangliu]

If the worker does not expose port, how can they Opens a socket and handshakes with all other workers.

In fact, in a container, even port is not exposed explicitly. we can still visit it by "podIP:port". workers/master use these sockets by "podIP:port"

[gaocegege]

@hougangliu

Yeah, I know. I am wondering what will happen if there is a worker pod failed. The IP will be changed.

[hougangliu]

Yeah, I know. I am wondering what will happen if there is a worker pod failed. The IP will be changed.

Once when a worker Pod failed, the PytorchJob restarts.

[gaocegege]

Yeah, the operator will restart the pod, but the pod IP may change. Can we handle the IP change in the PR?

[hougangliu]

Yeah, the operator will restart the pod, but the pod IP may change. Can we handle the IP change in the PR?

We don't need handle it. When new Pod created, it will register its NewPodIP:NewPort to MASTER_ADDR (master service name): MASTER_PORT by call Initialization method in pytorch source code.

[gaocegege]

OK, then LGTM.

/lgtm

I am not familiar with Distributed Training in PyTorch.

So, /assign @johnugeorge

[johnugeorge]

Just wondering if you have faced some issue without this PR? Or is it just a cleanup?

@gaocegege How is this different from distributed TF?

[hougangliu]

Just wondering if you have faced some issue without this PR? Or is it just a cleanup?

@gaocegege How is this different from distributed TF?

This is just for cleanup, all worker services in fact are unused. We can drop them.
tensorflow builds distribution-cluster by hostname+port list explicitly, but for pytorch, they only need a service (MASTER_ADDR: MASTER_PORT) for workers, all workers will register itself to the service to generate a cluster. They build cluster in different ways

[gaocegege]

tensorflow builds distribution-cluster by hostname+port list explicitly, but for pytorch, they only need a service (MASTER_ADDR: MASTER_PORT) for workers, all workers will register itself to the service to generate a cluster. They build cluster in different ways

I think it is the difference. @johnugeorge

[johnugeorge]

Got it.

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [johnugeorge]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment