diff options
author | Harald Musum <musum@yahooinc.com> | 2024-01-17 21:51:59 +0100 |
---|---|---|
committer | Harald Musum <musum@yahooinc.com> | 2024-01-17 21:51:59 +0100 |
commit | b894173caeb89b8f6850cae5e25641cb2a527e25 (patch) | |
tree | 2d42a3a3717a1c3c27f339a99b69f0c8d1482468 /config-model/src | |
parent | d451469633efc97687e30b33cae72e91c7e8e377 (diff) |
Only validate when on hosted vespa
Change tests to run on hosted Vespa and alter node resources and onnx cost
so that tests actually test what we want to test
Diffstat (limited to 'config-model/src')
2 files changed, 102 insertions, 31 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java index 373bfe24984..7fccd1c453f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java @@ -34,7 +34,8 @@ public class RestartOnDeployForOnnxModelChangesValidator implements ChangeValida @Override public void validate(ChangeContext context) { - if ( ! context.deployState().featureFlags().restartOnDeployWhenOnnxModelChanges()) return; + if ( ! context.deployState().featureFlags().restartOnDeployWhenOnnxModelChanges() + || ! context.deployState().isHosted()) return; // Compare onnx models used by each cluster and set restart on deploy for cluster if estimated cost, // model hash or model options have changed diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java index 67e8a4d512e..92c7fffd72f 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java @@ -2,11 +2,16 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ApplicationFile; +import com.yahoo.config.model.api.ApplicationClusterEndpoint; import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.api.ContainerEndpoint; import com.yahoo.config.model.api.OnnxModelCost; import com.yahoo.config.model.api.OnnxModelOptions; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; +import com.yahoo.config.model.provision.InMemoryProvisioner; +import com.yahoo.config.model.test.MockApplicationPackage; +import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; @@ -16,6 +21,7 @@ import java.net.URI; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -25,6 +31,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue; */ public class RestartOnDeployForOnnxModelChangesValidatorTest { + // Must be so large that changing model set or options requires restart (due to using more memory than available), + // but not so large that deployment will not work at all with one model + private static final long defaultCost = 723456789; + private static final long defaultHash = 0; + + @Test void validate_no_changes() { VespaModel current = createModel(); @@ -35,48 +47,64 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { @Test void validate_changed_estimated_cost() { - VespaModel current = createModel(); - VespaModel next = createModel(onnxModelCost(123, 0)); + VespaModel current = createModel(onnxModelCost(70000000, defaultHash)); + VespaModel next = createModel(onnxModelCost(723456789, defaultHash)); List<ConfigChangeAction> result = validateModel(current, next); assertEquals(1, result.size()); assertTrue(result.get(0).validationId().isEmpty()); - assertEquals("Onnx model 'https://my/url/e5-base-v2.onnx' has changed (estimated cost), need to restart services in container cluster 'cluster1'", result.get(0).getMessage()); - assertStartsWith("Onnx model 'https://my/url/e5-base-v2.onnx' has changed (estimated cost)", result); + assertEquals("Onnx model 'https://data.vespa.oath.cloud/onnx_models/e5-base-v2/model.onnx' has changed (estimated cost), need to restart services in container cluster 'cluster1'", result.get(0).getMessage()); + } + + @Test + void validate_changed_estimated_cost_non_hosted() { + boolean hosted = false; + VespaModel current = createModel(onnxModelCost(70000000, defaultHash), hosted); + VespaModel next = createModel(onnxModelCost(723456789, defaultHash), hosted); + List<ConfigChangeAction> result = validateModel(current, next, hosted); + assertEquals(0, result.size()); } @Test void validate_changed_hash() { VespaModel current = createModel(); - VespaModel next = createModel(onnxModelCost(0, 123)); + VespaModel next = createModel(onnxModelCost(defaultCost, 123)); List<ConfigChangeAction> result = validateModel(current, next); assertEquals(1, result.size()); - assertStartsWith("Onnx model 'https://my/url/e5-base-v2.onnx' has changed (model hash)", result); + assertStartsWith("Onnx model 'https://data.vespa.oath.cloud/onnx_models/e5-base-v2/model.onnx' has changed (model hash)", result); } @Test void validate_changed_option() { VespaModel current = createModel(); - VespaModel next = createModel(onnxModelCost(0, 0), "sequential"); + VespaModel next = createModel(onnxModelCost(), true, "sequential"); List<ConfigChangeAction> result = validateModel(current, next); assertEquals(1, result.size()); - assertStartsWith("Onnx model 'https://my/url/e5-base-v2.onnx' has changed (model option(s))", result); + assertStartsWith("Onnx model 'https://data.vespa.oath.cloud/onnx_models/e5-base-v2/model.onnx' has changed (model option(s))", result); } @Test void validate_changed_model_set() { VespaModel current = createModel(); - VespaModel next = createModel(onnxModelCost(0, 0), "parallel", "e5-small-v2"); + VespaModel next = createModel(onnxModelCost(), true, "parallel", "e5-small-v2"); List<ConfigChangeAction> result = validateModel(current, next); assertEquals(1, result.size()); - assertStartsWith("Onnx model set has changed from [https://my/url/e5-base-v2.onnx] to [https://my/url/e5-small-v2.onnx", result); + assertStartsWith("Onnx model set has changed from [https://data.vespa.oath.cloud/onnx_models/e5-base-v2/model.onnx] " + + "to [https://data.vespa.oath.cloud/onnx_models/e5-small-v2/model.onnx", + result); } private static List<ConfigChangeAction> validateModel(VespaModel current, VespaModel next) { - return ValidationTester.validateChanges(new RestartOnDeployForOnnxModelChangesValidator(), next, deployStateBuilder().previousModel(current).build()); + return validateModel(current, next, true); + } + + private static List<ConfigChangeAction> validateModel(VespaModel current, VespaModel next, boolean hosted) { + return ValidationTester.validateChanges(new RestartOnDeployForOnnxModelChangesValidator(), + next, + deployStateBuilder(hosted).previousModel(current).build()); } private static OnnxModelCost onnxModelCost() { - return onnxModelCost(0, 0); + return onnxModelCost(defaultCost, defaultHash); } private static OnnxModelCost onnxModelCost(long estimatedCost, long hash) { @@ -111,25 +139,65 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { } private static VespaModel createModel() { - return createModel(onnxModelCost()); + return createModel(onnxModelCost(), true); } private static VespaModel createModel(OnnxModelCost onnxModelCost) { - return createModel(onnxModelCost, "parallel"); + return createModel(onnxModelCost, true, "parallel"); + } + + private static VespaModel createModel(OnnxModelCost onnxModelCost, boolean hosted) { + return createModel(onnxModelCost, hosted, "parallel"); + } + + private static VespaModel createModel(OnnxModelCost onnxModelCost, boolean hosted, String executionMode) { + return createModel(onnxModelCost, hosted, executionMode, "e5-base-v2"); } - private static VespaModel createModel(OnnxModelCost onnxModelCost, String executionMode) { - return createModel(onnxModelCost, executionMode, "e5-base-v2"); + private static VespaModel createModel(OnnxModelCost onnxModelCost, boolean hosted, String executionMode, String modelId) { + DeployState.Builder builder = deployStateBuilder(hosted) + .onnxModelCost(onnxModelCost); + return hosted ? hostedModel(builder, executionMode, modelId) : nonHostedModel(builder, executionMode, modelId); } - private static VespaModel createModel(OnnxModelCost onnxModelCost, String executionMode, String modelId) { - DeployState.Builder builder = deployStateBuilder(); - builder.onnxModelCost(onnxModelCost); - return createModel(builder, executionMode, modelId); + private static VespaModel hostedModel(DeployState.Builder builder, String executionMode, String modelId) { + var servicesXml = """ + <services version='1.0'> + <container id='cluster1' version='1.0'> + <component id="hf-embedder" type="hugging-face-embedder"> + <transformer-model model-id="%s" url="https://my/url/%s.onnx"/> + <tokenizer-model model-id="e5-base-v2-vocab" path="app/tokenizer.json"/> + <onnx-execution-mode>%s</onnx-execution-mode> + </component> + <nodes count='1'> + <resources vcpu='1' memory='2Gb' disk='25Gb'/> + </nodes> + </container> + </services> + """.formatted(modelId, modelId, executionMode); + + var deploymentXml = """ + <deployment version='1.0' empty-host-ttl='1d'> + <instance id='default'> + <prod> + <region>us-east-1</region> + <region empty-host-ttl='0m'>us-north-1</region> + <region>us-west-1</region> + </prod> + </instance> + </deployment> + """; + + var applicationPackage = new MockApplicationPackage.Builder() + .withServices(servicesXml) + .withDeploymentSpec(deploymentXml) + .build(); + + return new VespaModelCreatorWithMockPkg(applicationPackage).create(builder); } - private static VespaModel createModel(DeployState.Builder builder, String executionMode, String modelId) { - String xml = """ + private static VespaModel nonHostedModel(DeployState.Builder builder, String executionMode, String modelId) { + var xml = """ <services version='1.0'> <container id='cluster1' version='1.0'> <http> @@ -141,20 +209,22 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { <onnx-execution-mode>%s</onnx-execution-mode> </component> </container> - <container id='cluster2' version='1.0'> - <http> - <server id='server1' port='8081'/> - </http> - </container> </services> """.formatted(modelId, modelId, executionMode); return new VespaModelCreatorWithMockPkg(null, xml).create(builder); } - private static DeployState.Builder deployStateBuilder() { - return new DeployState.Builder() - .properties((new TestProperties()).setRestartOnDeployForOnnxModelChanges(true)); + + private static DeployState.Builder deployStateBuilder(boolean hosted) { + var builder = new DeployState.Builder() + .properties((new TestProperties()) + .setRestartOnDeployForOnnxModelChanges(true) + .setHostedVespa(hosted)); + if (hosted) + builder.endpoints(Set.of(new ContainerEndpoint("cluster1", ApplicationClusterEndpoint.Scope.zone, List.of("tc.example.com")))) + .modelHostProvisioner(new InMemoryProvisioner(5, new NodeResources(1, 2, 25, 0.3), true)); + return builder; } private static void assertStartsWith(String expected, List<ConfigChangeAction> result) { |