You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@submarine.apache.org by pi...@apache.org on 2021/07/25 08:39:26 UTC
[submarine] branch master updated: SUBMARINE-942. Make experiment
ID consistent with TFJob and PyTorch Job
This is an automated email from the ASF dual-hosted git repository.
pingsutw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/submarine.git
The following commit(s) were added to refs/heads/master by this push:
new 6105358 SUBMARINE-942. Make experiment ID consistent with TFJob and PyTorch Job
6105358 is described below
commit 610535806c6208e53dbe0c2889cc944cf32835c5
Author: Kai-Hsun Chen <b0...@ntu.edu.tw>
AuthorDate: Thu Jul 22 11:25:26 2021 +0800
SUBMARINE-942. Make experiment ID consistent with TFJob and PyTorch Job
### What is this PR for?
1. Please refer to the following two JIRA issues.
* https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-886?filter=reportedbyme
* https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-880?filter=reportedbyme
In this JIRA issue, we need to make the experiment ID consistent with the name of TFJob and PyTorch Job. The difference is caused by this [link](https://github.com/apache/submarine/commit/8da9f478de9323dd098d06bbd52afdda3a27ce07#diff-c1cad0951ad3663eb075d964171d74ae94a35bec2b79f8c9894030c0446a6b99R119). Update ExperimentId.java.
2. Update [IntegrationTestK8s.md](http://submarine.apache.org/docs/devDocs/IntegrationTestK8s)
* "submarine-server-core" is a dependency package specified in pom.xml of test-k8s.
* In the document [BuildFromCode.md](http://submarine.apache.org/docs/devDocs/BuildFromCode), the command to build Submarine is `mvn clean package -DskipTests`. However, the package will be installed into the local repository at **install phase**, a later phase than both **package and verify phases**.
* Hence, we need to execute `mvn install -DskipTests` to ensure that the **test-k8s** module uses the latest "submarine-server-core" module.
3. Remove the field `name` in Experiment.java because the value of `name` is the same as `experimentId`.
### What type of PR is it?
[Improvement]
### Todos
### What is the Jira issue?
https://issues.apache.org/jira/browse/SUBMARINE-942
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* Do the license files need updating? No
* Are there breaking changes for older versions? No
* Does this need new documentation? No
Author: Kai-Hsun Chen <b0...@ntu.edu.tw>
Signed-off-by: Kevin <pi...@apache.org>
Closes #683 from kevin85421/SUBMARINE-942 and squashes the following commits:
594e46ff [Kai-Hsun Chen] Remove experiment name
f051ff87 [Kai-Hsun Chen] Update IntegrationTestK8s.md
c88a384a [Kai-Hsun Chen] Refactor
b81c2daa [Kai-Hsun Chen] Refactor
495ceb32 [Kai-Hsun Chen] SUBMARINE-942. Make experiment ID consistent with TFJob and PyTorch Job
ffd2c168 [Kai-Hsun Chen] SUBMARINE-942. Make experiment ID consistent with TFJob and PyTorch Job
---
.../server/api/experiment/Experiment.java | 21 ++-------------------
.../server/api/experiment/ExperimentId.java | 12 ++++++------
.../server/experiment/ExperimentManager.java | 3 +--
.../server/gson/ExperimentIdDeserializer.java | 1 -
.../server/experiment/ExperimentManagerTest.java | 1 -
.../server/rest/ExperimentRestApiTest.java | 2 --
.../server/submitter/k8s/util/MLJobConverter.java | 2 --
.../server/submitter/k8s/K8SJobSubmitterTest.java | 2 --
.../apache/submarine/rest/ExperimentRestApiIT.java | 15 ++++++---------
.../rest/ExperimentTemplateManagerRestApiIT.java | 3 +--
website/docs/devDocs/IntegrationTestK8s.md | 22 ++++++++++++----------
11 files changed, 28 insertions(+), 56 deletions(-)
diff --git a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/Experiment.java b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/Experiment.java
index 2f14460..ff080b7 100644
--- a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/Experiment.java
+++ b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/Experiment.java
@@ -26,7 +26,6 @@ import org.apache.submarine.server.api.spec.ExperimentSpec;
*/
public class Experiment {
private ExperimentId experimentId;
- private String name;
private String uid;
private String status;
private String acceptedTime;
@@ -52,22 +51,6 @@ public class Experiment {
}
/**
- * Get the job name which specified by user through the JobSpec
- * @return the job name
- */
- public String getName() {
- return name;
- }
-
- /**
- * Set the job name which specified by user
- * @param name job name
- */
- public void setName(String name) {
- this.name = name;
- }
-
- /**
* The unique identifier for the job, used to retire the job info from the cluster management
* system.
*
@@ -159,8 +142,8 @@ public class Experiment {
public void rebuild(Experiment experiment) {
if (experiment != null) {
- if (experiment.getName() != null) {
- this.setName(experiment.getName());
+ if (experiment.getExperimentId() != null) {
+ this.setExperimentId(experiment.getExperimentId());
}
if (experiment.getUid() != null) {
this.setUid(experiment.getUid());
diff --git a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/ExperimentId.java b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/ExperimentId.java
index 42be1b1..cc5fa5e 100644
--- a/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/ExperimentId.java
+++ b/submarine-server/server-api/src/main/java/org/apache/submarine/server/api/experiment/ExperimentId.java
@@ -22,12 +22,12 @@ package org.apache.submarine.server.api.experiment;
import org.apache.submarine.commons.utils.AbstractUniqueIdGenerator;
/**
- * The unique id for experiment. Formatter: experiment_${server_timestamp}_${counter}
- * Such as: experiment_1577627710_0001
+ * The unique id for experiment. Formatter: experiment-${server_timestamp}-${counter}
+ * Such as: experiment-1577627710-0001
*/
public class ExperimentId extends AbstractUniqueIdGenerator<ExperimentId> {
- private static final String EXPERIMENT_ID_PREFIX = "experiment_";
-
+ private static final String EXPERIMENT_ID_PREFIX = "experiment-";
+
/**
* Get the object of JobId.
* @param jobId job id string
@@ -37,7 +37,7 @@ public class ExperimentId extends AbstractUniqueIdGenerator<ExperimentId> {
if (jobId == null) {
return null;
}
- String[] components = jobId.split("\\_");
+ String[] components = jobId.split("\\-");
if (components.length != 3) {
return null;
}
@@ -60,7 +60,7 @@ public class ExperimentId extends AbstractUniqueIdGenerator<ExperimentId> {
@Override
public String toString() {
StringBuilder sb = new StringBuilder(64);
- sb.append(EXPERIMENT_ID_PREFIX).append(getServerTimestamp()).append("_");
+ sb.append(EXPERIMENT_ID_PREFIX).append(getServerTimestamp()).append("-");
format(sb, getId());
return sb.toString();
}
diff --git a/submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java b/submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
index cc57dca..7ccf35b 100644
--- a/submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
+++ b/submarine-server/server-core/src/main/java/org/apache/submarine/server/experiment/ExperimentManager.java
@@ -116,8 +116,7 @@ public class ExperimentManager {
String lowerName = spec.getMeta().getName().toLowerCase();
spec.getMeta().setName(lowerName);
- spec.getMeta().setExperimentId(id.toString().replaceAll("_", "-"));
- LOG.info(spec.getMeta().getExperimentId());
+ spec.getMeta().setExperimentId(id.toString());
Experiment experiment = submitter.createExperiment(spec);
experiment.setExperimentId(id);
diff --git a/submarine-server/server-core/src/main/java/org/apache/submarine/server/gson/ExperimentIdDeserializer.java b/submarine-server/server-core/src/main/java/org/apache/submarine/server/gson/ExperimentIdDeserializer.java
index 74c047e..1b7718e 100644
--- a/submarine-server/server-core/src/main/java/org/apache/submarine/server/gson/ExperimentIdDeserializer.java
+++ b/submarine-server/server-core/src/main/java/org/apache/submarine/server/gson/ExperimentIdDeserializer.java
@@ -28,7 +28,6 @@ import org.apache.submarine.server.api.experiment.ExperimentId;
import java.lang.reflect.Type;
public class ExperimentIdDeserializer implements JsonDeserializer<ExperimentId> {
-
@Override
public ExperimentId deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context)
throws JsonParseException {
diff --git a/submarine-server/server-core/src/test/java/org/apache/submarine/server/experiment/ExperimentManagerTest.java b/submarine-server/server-core/src/test/java/org/apache/submarine/server/experiment/ExperimentManagerTest.java
index 578f86d..d70639b 100644
--- a/submarine-server/server-core/src/test/java/org/apache/submarine/server/experiment/ExperimentManagerTest.java
+++ b/submarine-server/server-core/src/test/java/org/apache/submarine/server/experiment/ExperimentManagerTest.java
@@ -224,7 +224,6 @@ public class ExperimentManagerTest {
assertEquals(expected.getCreatedTime(), actual.getCreatedTime());
assertEquals(expected.getRunningTime(), actual.getRunningTime());
assertEquals(expected.getAcceptedTime(), actual.getAcceptedTime());
- assertEquals(expected.getName(), actual.getName());
assertEquals(expected.getStatus(), actual.getStatus());
assertEquals(expected.getExperimentId(), actual.getExperimentId());
assertEquals(expected.getFinishedTime(), actual.getFinishedTime());
diff --git a/submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java b/submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java
index 5ffff37..38be2cc 100644
--- a/submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java
+++ b/submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java
@@ -109,7 +109,6 @@ public class ExperimentRestApiTest {
actualExperiment.setRunningTime(experimentRunningTime);
actualExperiment.setFinishedTime(experimentFinishedTime);
actualExperiment.setUid(experimentUid);
- actualExperiment.setName(experimentName);
actualExperiment.setStatus(experimentStatus);
actualExperiment.setExperimentId(experimentId);
kernelSpec.setName(kernelSpecName);
@@ -220,7 +219,6 @@ public class ExperimentRestApiTest {
assertEquals(experimentCreatedTime, experiment.getCreatedTime());
assertEquals(experimentRunningTime, experiment.getRunningTime());
assertEquals(experimentAcceptedTime, experiment.getAcceptedTime());
- assertEquals(experimentName, experiment.getName());
assertEquals(experimentStatus, experiment.getStatus());
assertEquals(experimentId, experiment.getExperimentId());
assertEquals(experimentFinishedTime, experiment.getFinishedTime());
diff --git a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java
index 99decf1..725491c 100644
--- a/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java
+++ b/submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java
@@ -39,7 +39,6 @@ public class MLJobConverter {
public static Experiment toJobFromMLJob(MLJob mlJob) {
Experiment experiment = new Experiment();
experiment.setUid(mlJob.getMetadata().getUid());
- experiment.setName(mlJob.getMetadata().getName());
DateTime dateTime = mlJob.getMetadata().getCreationTimestamp();
if (dateTime != null) {
experiment.setAcceptedTime(dateTime.toString());
@@ -80,7 +79,6 @@ public class MLJobConverter {
V1StatusDetails details = status.getDetails();
if (details != null) {
experiment.setUid(details.getUid());
- experiment.setName(details.getName());
}
if (status.getStatus().toLowerCase().equals("success")) {
experiment.setStatus(Experiment.Status.STATUS_DELETED.getValue());
diff --git a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java
index ca8d0d8..5be013c 100644
--- a/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java
+++ b/submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java
@@ -113,7 +113,6 @@ public class K8SJobSubmitterTest extends SpecBuilder {
Experiment experimentFound = submitter.findExperiment(spec);
Assert.assertNotNull(experimentFound);
Assert.assertEquals(experimentCreated.getUid(), experimentFound.getUid());
- Assert.assertEquals(experimentCreated.getName(), experimentFound.getName());
Assert.assertEquals(experimentCreated.getAcceptedTime(), experimentFound.getAcceptedTime());
// delete
@@ -121,6 +120,5 @@ public class K8SJobSubmitterTest extends SpecBuilder {
Assert.assertNotNull(experimentDeleted);
Assert.assertEquals(Experiment.Status.STATUS_DELETED.getValue(), experimentDeleted.getStatus());
Assert.assertEquals(experimentFound.getUid(), experimentDeleted.getUid());
- Assert.assertEquals(experimentFound.getName(), experimentDeleted.getName());
}
}
diff --git a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentRestApiIT.java b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentRestApiIT.java
index 4f247d3..c9dc564 100644
--- a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentRestApiIT.java
+++ b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentRestApiIT.java
@@ -202,10 +202,8 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest {
String json = postMethod.getResponseBodyAsString();
JsonResponse jsonResponse = gson.fromJson(json, JsonResponse.class);
Assert.assertEquals(Response.Status.OK.getStatusCode(), jsonResponse.getCode());
-
Experiment createdExperiment = gson.fromJson(gson.toJson(jsonResponse.getResult()), Experiment.class);
verifyCreateJobApiResult(createdExperiment);
-
// find
GetMethod getMethod = httpGet(BASE_API_PATH + "/" + createdExperiment.getExperimentId().toString());
Assert.assertEquals(Response.Status.OK.getStatusCode(), getMethod.getStatusCode());
@@ -225,8 +223,7 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest {
// https://tools.ietf.org/html/rfc5789
// delete
- DeleteMethod deleteMethod = httpDelete(
- BASE_API_PATH + "/" + createdExperiment.getExperimentId().toString());
+ DeleteMethod deleteMethod = httpDelete(BASE_API_PATH + "/" + createdExperiment.getExperimentId().toString());
Assert.assertEquals(Response.Status.OK.getStatusCode(), deleteMethod.getStatusCode());
json = deleteMethod.getResponseBodyAsString();
@@ -307,7 +304,7 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest {
Experiment createdExperiment, Experiment foundExperiment) throws Exception {
Assert.assertEquals(createdExperiment.getExperimentId(), foundExperiment.getExperimentId());
Assert.assertEquals(createdExperiment.getUid(), foundExperiment.getUid());
- Assert.assertEquals(createdExperiment.getName(), foundExperiment.getName());
+ Assert.assertEquals(createdExperiment.getExperimentId().toString(), foundExperiment.getExperimentId().toString());
Assert.assertEquals(createdExperiment.getAcceptedTime(), foundExperiment.getAcceptedTime());
assertK8sResultEquals(foundExperiment);
@@ -317,7 +314,7 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest {
KfOperator operator = kfOperatorMap.get(experiment.getSpec().getMeta().getFramework().toLowerCase());
JsonObject rootObject =
getJobByK8sApi(operator.getGroup(), operator.getVersion(),
- operator.getNamespace(), operator.getPlural(), experiment.getName());
+ operator.getNamespace(), operator.getPlural(), experiment.getExperimentId().toString());
JsonArray actualCommand = (JsonArray) rootObject.getAsJsonObject("spec")
.getAsJsonObject("tfReplicaSpecs").getAsJsonObject("Worker")
.getAsJsonObject("template").getAsJsonObject("spec")
@@ -389,7 +386,7 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest {
private void assertK8sResultEquals(Experiment experiment) throws Exception {
KfOperator operator = kfOperatorMap.get(experiment.getSpec().getMeta().getFramework().toLowerCase());
JsonObject rootObject = getJobByK8sApi(operator.getGroup(), operator.getVersion(),
- operator.getNamespace(), operator.getPlural(), experiment.getName());
+ operator.getNamespace(), operator.getPlural(), experiment.getExperimentId().toString());
JsonObject metadataObject = rootObject.getAsJsonObject("metadata");
String uid = metadataObject.getAsJsonPrimitive("uid").getAsString();
@@ -407,7 +404,7 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest {
}
private void verifyDeleteJobApiResult(Experiment createdExperiment, Experiment deletedExperiment) {
- Assert.assertEquals(createdExperiment.getName(), deletedExperiment.getName());
+ Assert.assertEquals(createdExperiment.getExperimentId().toString(), deletedExperiment.getExperimentId().toString());
Assert.assertEquals(Experiment.Status.STATUS_DELETED.getValue(), deletedExperiment.getStatus());
// verify the result by K8s api
@@ -416,7 +413,7 @@ public class ExperimentRestApiIT extends AbstractSubmarineServerTest {
JsonObject rootObject = null;
try {
rootObject = getJobByK8sApi(operator.getGroup(), operator.getVersion(),
- operator.getNamespace(), operator.getPlural(), createdExperiment.getName());
+ operator.getNamespace(), operator.getPlural(), createdExperiment.getExperimentId().toString());
} catch (ApiException e) {
Assert.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), e.getCode());
} finally {
diff --git a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java
index b0d7b75..f902ac7 100644
--- a/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java
+++ b/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/ExperimentTemplateManagerRestApiIT.java
@@ -213,9 +213,8 @@ public class ExperimentTemplateManagerRestApiIT extends AbstractSubmarineServerT
LOG.info(gson.toJson(jsonResponse.getResult()));
Experiment experiment = gson.fromJson(gson.toJson(jsonResponse.getResult()), Experiment.class);
-
DeleteMethod deleteMethod = httpDelete("/api/" + RestConstants.V1 + "/" + RestConstants.EXPERIMENT + "/"
- + experiment.getExperimentId().toString());
+ + experiment.getExperimentId().toString());
Assert.assertEquals(Response.Status.OK.getStatusCode(), deleteMethod.getStatusCode());
json = deleteMethod.getResponseBodyAsString();
diff --git a/website/docs/devDocs/IntegrationTestK8s.md b/website/docs/devDocs/IntegrationTestK8s.md
index d486a74..2754318 100644
--- a/website/docs/devDocs/IntegrationTestK8s.md
+++ b/website/docs/devDocs/IntegrationTestK8s.md
@@ -32,16 +32,18 @@ title: How to Run Integration K8s Test
2. Build the submarine from source and upgrade the server pod through this [`guide`](./Development/#build-from-source)
3. Forward port
-
- ```bash
- kubectl port-forward --address 0.0.0.0 service/submarine-traefik 8080:80
- ```
-
-4. Execute the test command
-
- ```bash
- mvn verify -DskipRat -pl :submarine-test-k8s -Phadoop-2.9 -B
- ```
+ ```bash
+ kubectl port-forward --address 0.0.0.0 service/submarine-traefik 8080:80
+ ```
+4. Install the latest package "submarine-server-core" into the local repository, for use as a dependency in the module `test-k8s`
+ ```bash
+ mvn install -DskipTests
+ ```
+
+5. Execute the test command
+ ```bash
+ mvn verify -DskipRat -pl :submarine-test-k8s -Phadoop-2.9 -B
+ ```
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@submarine.apache.org
For additional commands, e-mail: dev-help@submarine.apache.org