From 50da5391abb202d31a0549e05846912ac5f299e3 Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Mon, 22 Jan 2024 17:11:44 -0800 Subject: [PATCH 01/13] Explicitly set default dependency upgrade strategy to only-if-needed --- .../src/main/java/org/pytorch/serve/wlm/ModelManager.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java index 435c3b9415..03a8954010 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java @@ -228,6 +228,8 @@ private void setupModelDependencies(Model model) commandParts.add("pip"); commandParts.add("install"); commandParts.add("-U"); + commandParts.add("--upgrade-strategy"); + commandParts.add("only-if-needed"); commandParts.add("-t"); commandParts.add(dependencyPath.getAbsolutePath()); commandParts.add("-r"); From 254518f7ba9f425186d2a4156faeef29d57548df Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Thu, 25 Jan 2024 15:06:47 -0800 Subject: [PATCH 02/13] Add support for venv creation per model --- .../serve/util/messages/EnvironmentUtils.java | 15 ++ .../org/pytorch/serve/wlm/ModelManager.java | 141 ++++++++++++------ 2 files changed, 112 insertions(+), 44 deletions(-) diff --git a/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java b/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java index 7b87b8e278..1d2fac69a1 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java +++ b/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java @@ -3,6 +3,8 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.HashMap; import java.util.Map; @@ -77,12 +79,25 @@ public static String getPythonRunTime(Model model) { Manifest.RuntimeType runtime = model.getModelArchive().getManifest().getRuntime(); if (runtime == Manifest.RuntimeType.PYTHON) { pythonRuntime = configManager.getPythonExecutable(); + Path pythonVenvRuntime = Paths.get(getPythonVenvPath(model), "bin", "python"); + if (Files.exists(pythonVenvRuntime)) { + pythonRuntime = pythonVenvRuntime.toString(); + } } else { pythonRuntime = runtime.getValue(); } return pythonRuntime; } + public static String getPythonVenvPath(Model model) { + File modelDir = model.getModelDir(); + if (Files.isSymbolicLink(modelDir.toPath())) { + modelDir = modelDir.getParentFile(); + } + Path venvPath = Paths.get(modelDir.getAbsolutePath(), "venv"); + return venvPath.toString(); + } + public static String[] getCppEnvString(String libPath) { ArrayList envList = new ArrayList<>(); StringBuilder cppPath = new StringBuilder(); diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java index 03a8954010..1b6e9da935 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java @@ -6,7 +6,6 @@ import java.io.IOException; import java.io.InputStreamReader; import java.net.HttpURLConnection; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -100,6 +99,8 @@ public void registerAndUpdateModel(String modelName, JsonObject modelInfo) createVersionedModel(tempModel, versionId); + setupModelVenv(tempModel); + setupModelDependencies(tempModel); if (defaultVersion) { modelManager.setDefaultVersion(modelName, versionId); @@ -153,6 +154,8 @@ public ModelArchive registerModel( } } + setupModelVenv(tempModel); + setupModelDependencies(tempModel); logger.info("Model {} loaded.", tempModel.getModelName()); @@ -205,24 +208,82 @@ private ModelArchive createModelArchive( return archive; } + private void setupModelVenv(Model model) + throws IOException, InterruptedException, ModelException { + String venvPath = EnvironmentUtils.getPythonVenvPath(model); + List commandParts = new ArrayList<>(); + commandParts.add(EnvironmentUtils.getPythonRunTime(model)); + commandParts.add("-m"); + commandParts.add("venv"); + commandParts.add("--clear"); + commandParts.add("--system-site-packages"); + commandParts.add(venvPath); + + ProcessBuilder processBuilder = new ProcessBuilder(commandParts); + + if (isValidVenvPath(venvPath)) { + processBuilder.directory(Paths.get(venvPath).toFile().getParentFile()); + } else { + throw new ModelException( + "Invalid python venv path for model " + model.getModelName() + ": " + venvPath); + } + Map environment = processBuilder.environment(); + String[] envp = + EnvironmentUtils.getEnvString( + configManager.getModelServerHome(), + model.getModelDir().getAbsolutePath(), + null); + for (String envVar : envp) { + String[] parts = envVar.split("=", 2); + if (parts.length == 2) { + environment.put(parts[0], parts[1]); + } + } + processBuilder.redirectErrorStream(true); + + Process process = processBuilder.start(); + + int exitCode = process.waitFor(); + String line; + StringBuilder outputString = new StringBuilder(); + BufferedReader brdr = new BufferedReader(new InputStreamReader(process.getInputStream())); + while ((line = brdr.readLine()) != null) { + outputString.append(line); + } + + if (exitCode == 0) { + logger.info( + "Created virtual environment for model {}:\n{}", + model.getModelName(), + outputString.toString()); + } else { + logger.error( + "Virtual environment creation for model {} failed:\n{}", + model.getModelName(), + outputString.toString()); + throw new ModelException( + "Virtual environment creation failed for model " + model.getModelName()); + } + } + private void setupModelDependencies(Model model) throws IOException, InterruptedException, ModelException { String requirementsFile = model.getModelArchive().getManifest().getModel().getRequirementsFile(); if (configManager.getInstallPyDepPerModel() && requirementsFile != null) { - Path requirementsFilePath = - Paths.get(model.getModelDir().getAbsolutePath(), requirementsFile); - String pythonRuntime = EnvironmentUtils.getPythonRunTime(model); - - File dependencyPath = model.getModelDir(); - if (Files.isSymbolicLink(dependencyPath.toPath())) { - dependencyPath = dependencyPath.getParentFile(); + if (!isValidVenvPath(pythonRuntime)) { + throw new ModelException( + "Invalid python venv runtime path for model " + + model.getModelName() + + ": " + + pythonRuntime); } + Path requirementsFilePath = + Paths.get(model.getModelDir().getAbsolutePath(), requirementsFile); List commandParts = new ArrayList<>(); - commandParts.add(pythonRuntime); commandParts.add("-m"); commandParts.add("pip"); @@ -230,26 +291,16 @@ private void setupModelDependencies(Model model) commandParts.add("-U"); commandParts.add("--upgrade-strategy"); commandParts.add("only-if-needed"); - commandParts.add("-t"); - commandParts.add(dependencyPath.getAbsolutePath()); commandParts.add("-r"); commandParts.add(requirementsFilePath.toString()); + ProcessBuilder processBuilder = new ProcessBuilder(commandParts); + String[] envp = EnvironmentUtils.getEnvString( configManager.getModelServerHome(), model.getModelDir().getAbsolutePath(), null); - - ProcessBuilder processBuilder = new ProcessBuilder(commandParts); - if (isValidDependencyPath(dependencyPath)) { - processBuilder.directory(dependencyPath); - } else { - throw new ModelException( - "Invalid 3rd party package installation path " - + dependencyPath.getCanonicalPath()); - } - Map environment = processBuilder.environment(); for (String envVar : envp) { String[] parts = envVar.split("=", 2); @@ -257,37 +308,39 @@ private void setupModelDependencies(Model model) environment.put(parts[0], parts[1]); } } - Process process = processBuilder.start(); - int exitCode = process.waitFor(); - - if (exitCode != 0) { + processBuilder.directory( + Paths.get(EnvironmentUtils.getPythonVenvPath(model)).toFile().getParentFile()); + processBuilder.redirectErrorStream(true); - String line; - StringBuilder outputString = new StringBuilder(); - // process's stdout is InputStream for caller process - BufferedReader brdr = - new BufferedReader(new InputStreamReader(process.getInputStream())); - while ((line = brdr.readLine()) != null) { - outputString.append(line); - } - StringBuilder errorString = new StringBuilder(); - // process's stderr is ErrorStream for caller process - brdr = new BufferedReader(new InputStreamReader(process.getErrorStream())); - while ((line = brdr.readLine()) != null) { - errorString.append(line); - } + Process process = processBuilder.start(); - logger.error("Dependency installation stderr:\n" + errorString.toString()); + int exitCode = process.waitFor(); + String line; + StringBuilder outputString = new StringBuilder(); + BufferedReader brdr = + new BufferedReader(new InputStreamReader(process.getInputStream())); + while ((line = brdr.readLine()) != null) { + outputString.append(line); + } + if (exitCode == 0) { + logger.info( + "Installed custom dependencies for model {}:\n{}", + model.getModelName(), + outputString.toString()); + } else { + logger.error( + "Failed to install custom dependencies for model {}:\n{}", + model.getModelName(), + outputString.toString()); throw new ModelException( - "Custom pip package installation failed for " + model.getModelName()); + "Failed to install custom dependencies for model " + model.getModelName()); } } } - private boolean isValidDependencyPath(File dependencyPath) { - if (dependencyPath - .toPath() + private boolean isValidVenvPath(String venvPath) { + if (Paths.get(venvPath) .normalize() .startsWith(FileUtils.getTempDirectory().toPath().normalize())) { return true; From 999782eff6f16066aa0153b90340a0454c502b8a Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Fri, 26 Jan 2024 15:28:47 -0800 Subject: [PATCH 03/13] Create virtual env only when requirements need to be installed --- .../java/org/pytorch/serve/wlm/ModelManager.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java index 1b6e9da935..31215a5e7f 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java @@ -99,8 +99,6 @@ public void registerAndUpdateModel(String modelName, JsonObject modelInfo) createVersionedModel(tempModel, versionId); - setupModelVenv(tempModel); - setupModelDependencies(tempModel); if (defaultVersion) { modelManager.setDefaultVersion(modelName, versionId); @@ -154,8 +152,6 @@ public ModelArchive registerModel( } } - setupModelVenv(tempModel); - setupModelDependencies(tempModel); logger.info("Model {} loaded.", tempModel.getModelName()); @@ -253,13 +249,14 @@ private void setupModelVenv(Model model) if (exitCode == 0) { logger.info( - "Created virtual environment for model {}:\n{}", + "Created virtual environment for model {}: {}", model.getModelName(), - outputString.toString()); + venvPath); } else { logger.error( - "Virtual environment creation for model {} failed:\n{}", + "Virtual environment creation for model {} at {} failed:\n{}", model.getModelName(), + venvPath, outputString.toString()); throw new ModelException( "Virtual environment creation failed for model " + model.getModelName()); @@ -272,6 +269,7 @@ private void setupModelDependencies(Model model) model.getModelArchive().getManifest().getModel().getRequirementsFile(); if (configManager.getInstallPyDepPerModel() && requirementsFile != null) { + setupModelVenv(model); String pythonRuntime = EnvironmentUtils.getPythonRunTime(model); if (!isValidVenvPath(pythonRuntime)) { throw new ModelException( From e1c073497d9cda31a0a139722d3402371cc04494 Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Mon, 29 Jan 2024 10:27:46 -0800 Subject: [PATCH 04/13] Format logger and exception messages --- .../java/org/pytorch/serve/wlm/ModelManager.java | 14 ++++++-------- .../java/org/pytorch/serve/ModelServerTest.java | 2 +- .../test/java/org/pytorch/serve/WorkflowTest.java | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java index 6becb5bb10..5e059633e1 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java @@ -244,14 +244,12 @@ private void setupModelVenv(Model model) StringBuilder outputString = new StringBuilder(); BufferedReader brdr = new BufferedReader(new InputStreamReader(process.getInputStream())); while ((line = brdr.readLine()) != null) { - outputString.append(line); + outputString.append(line + "\n"); } if (exitCode == 0) { logger.info( - "Created virtual environment for model {}: {}", - model.getModelName(), - venvPath); + "Created virtual environment for model {}: {}", model.getModelName(), venvPath); } else { logger.error( "Virtual environment creation for model {} at {} failed:\n{}", @@ -318,21 +316,21 @@ private void setupModelDependencies(Model model) BufferedReader brdr = new BufferedReader(new InputStreamReader(process.getInputStream())); while ((line = brdr.readLine()) != null) { - outputString.append(line); + outputString.append(line + "\n"); } if (exitCode == 0) { logger.info( - "Installed custom dependencies for model {}:\n{}", + "Installed custom pip packages for model {}:\n{}", model.getModelName(), outputString.toString()); } else { logger.error( - "Failed to install custom dependencies for model {}:\n{}", + "Custom pip package installation failed for model {}:\n{}", model.getModelName(), outputString.toString()); throw new ModelException( - "Failed to install custom dependencies for model " + model.getModelName()); + "Custom pip package installation failed for model " + model.getModelName()); } } } diff --git a/frontend/server/src/test/java/org/pytorch/serve/ModelServerTest.java b/frontend/server/src/test/java/org/pytorch/serve/ModelServerTest.java index b28b8963bb..df67aa95df 100644 --- a/frontend/server/src/test/java/org/pytorch/serve/ModelServerTest.java +++ b/frontend/server/src/test/java/org/pytorch/serve/ModelServerTest.java @@ -1169,7 +1169,7 @@ public void testModelWithInvalidCustomPythonDependency() Assert.assertEquals(TestUtils.getHttpStatus(), HttpResponseStatus.BAD_REQUEST); Assert.assertEquals( resp.getMessage(), - "Custom pip package installation failed for custom_invalid_python_dep"); + "Custom pip package installation failed for model custom_invalid_python_dep"); TestUtils.setConfiguration(configManager, "install_py_dep_per_model", "false"); channel.close().sync(); } diff --git a/frontend/server/src/test/java/org/pytorch/serve/WorkflowTest.java b/frontend/server/src/test/java/org/pytorch/serve/WorkflowTest.java index 68a025065e..9c530d2f8b 100644 --- a/frontend/server/src/test/java/org/pytorch/serve/WorkflowTest.java +++ b/frontend/server/src/test/java/org/pytorch/serve/WorkflowTest.java @@ -424,7 +424,7 @@ public void testWorkflowWithInvalidCustomPythonDependencyModel() Assert.assertEquals(TestUtils.getHttpStatus(), HttpResponseStatus.INTERNAL_SERVER_ERROR); Assert.assertEquals( resp.getMessage(), - "Workflow custom_invalid_python_dep has failed to register. Failures: [Workflow Node custom_invalid_python_dep__custom_invalid_python_dep failed to register. Details: Custom pip package installation failed for custom_invalid_python_dep__custom_invalid_python_dep]"); + "Workflow custom_invalid_python_dep has failed to register. Failures: [Workflow Node custom_invalid_python_dep__custom_invalid_python_dep failed to register. Details: Custom pip package installation failed for model custom_invalid_python_dep__custom_invalid_python_dep]"); TestUtils.setConfiguration(configManager, "install_py_dep_per_model", "false"); channel.close().sync(); } From d7e72c860c98bc2613efdf449d82f2e03c038fde Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Fri, 2 Feb 2024 12:24:06 -0800 Subject: [PATCH 05/13] Enable per model useVenv configuration option --- .../serve/archive/model/ModelConfig.java | 22 ++- .../serve/util/messages/EnvironmentUtils.java | 6 +- .../org/pytorch/serve/wlm/ModelManager.java | 137 +++++++++++------- 3 files changed, 114 insertions(+), 51 deletions(-) diff --git a/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java b/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java index adcf42afd8..832391141c 100644 --- a/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java +++ b/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java @@ -65,9 +65,14 @@ public class ModelConfig { private int maxSequenceJobQueueSize = 1; /** the max number of sequences can be accepted. The default value is 1. */ private int maxNumSequence = 1; - /** continuousBatching is a flag to enable continuous batching. */ private boolean continuousBatching; + /** + * Create python virtual environment when using python backend to: 1) Install model dependencies + * (if enabled globally using install_py_dep_per_model=true) 2) Run workers for model loading + * and inference + */ + private boolean useVenv; public static ModelConfig build(Map yamlMap) { ModelConfig modelConfig = new ModelConfig(); @@ -207,6 +212,13 @@ public static ModelConfig build(Map yamlMap) { v); } break; + case "useVenv": + if (v instanceof Boolean) { + modelConfig.setUseVenv((boolean) v); + } else { + logger.warn("Invalid useVenv: {}, should be true or false", v); + } + break; default: break; } @@ -379,6 +391,14 @@ public void setMaxNumSequence(int maxNumSequence) { this.maxNumSequence = Math.max(1, maxNumSequence); } + public boolean getUseVenv() { + return useVenv; + } + + public void setUseVenv(boolean useVenv) { + this.useVenv = useVenv; + } + public enum ParallelType { NONE(""), PP("pp"), diff --git a/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java b/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java index 1d2fac69a1..43708c0bc3 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java +++ b/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.regex.Pattern; import org.pytorch.serve.archive.model.Manifest; +import org.pytorch.serve.archive.model.ModelConfig; import org.pytorch.serve.util.ConfigManager; import org.pytorch.serve.wlm.Model; import org.slf4j.Logger; @@ -79,8 +80,11 @@ public static String getPythonRunTime(Model model) { Manifest.RuntimeType runtime = model.getModelArchive().getManifest().getRuntime(); if (runtime == Manifest.RuntimeType.PYTHON) { pythonRuntime = configManager.getPythonExecutable(); + ModelConfig modelConfig = model.getModelArchive().getModelConfig(); Path pythonVenvRuntime = Paths.get(getPythonVenvPath(model), "bin", "python"); - if (Files.exists(pythonVenvRuntime)) { + if (modelConfig != null + && modelConfig.getUseVenv() == true + && Files.exists(pythonVenvRuntime)) { pythonRuntime = pythonVenvRuntime.toString(); } } else { diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java index 5e059633e1..9c8afaeb52 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java @@ -6,6 +6,7 @@ import java.io.IOException; import java.io.InputStreamReader; import java.net.HttpURLConnection; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -99,6 +100,8 @@ public void registerAndUpdateModel(String modelName, JsonObject modelInfo) createVersionedModel(tempModel, versionId); + setupModelVenv(tempModel); + setupModelDependencies(tempModel); if (defaultVersion) { modelManager.setDefaultVersion(modelName, versionId); @@ -152,6 +155,8 @@ public ModelArchive registerModel( } } + setupModelVenv(tempModel); + setupModelDependencies(tempModel); logger.info("Model {} loaded.", tempModel.getModelName()); @@ -206,9 +211,16 @@ private ModelArchive createModelArchive( private void setupModelVenv(Model model) throws IOException, InterruptedException, ModelException { + ModelConfig modelConfig = model.getModelArchive().getModelConfig(); + if (model.getModelArchive().getManifest().getRuntime() != Manifest.RuntimeType.PYTHON + || modelConfig == null + || modelConfig.getUseVenv() != true) { + return; + } + String venvPath = EnvironmentUtils.getPythonVenvPath(model); List commandParts = new ArrayList<>(); - commandParts.add(EnvironmentUtils.getPythonRunTime(model)); + commandParts.add(configManager.getPythonExecutable()); commandParts.add("-m"); commandParts.add("venv"); commandParts.add("--clear"); @@ -217,7 +229,7 @@ private void setupModelVenv(Model model) ProcessBuilder processBuilder = new ProcessBuilder(commandParts); - if (isValidVenvPath(venvPath)) { + if (isValidDependencyPath(venvPath)) { processBuilder.directory(Paths.get(venvPath).toFile().getParentFile()); } else { throw new ModelException( @@ -266,10 +278,19 @@ private void setupModelDependencies(Model model) String requirementsFile = model.getModelArchive().getManifest().getModel().getRequirementsFile(); - if (configManager.getInstallPyDepPerModel() && requirementsFile != null) { - setupModelVenv(model); - String pythonRuntime = EnvironmentUtils.getPythonRunTime(model); - if (!isValidVenvPath(pythonRuntime)) { + if (!configManager.getInstallPyDepPerModel() || requirementsFile == null) { + return; + } + + ModelConfig modelConfig = model.getModelArchive().getModelConfig(); + String pythonRuntime = EnvironmentUtils.getPythonRunTime(model); + Path requirementsFilePath = + Paths.get(model.getModelDir().getAbsolutePath(), requirementsFile); + List commandParts = new ArrayList<>(); + ProcessBuilder processBuilder = new ProcessBuilder(); + + if (modelConfig != null && modelConfig.getUseVenv() == true) { + if (!isValidDependencyPath(pythonRuntime)) { throw new ModelException( "Invalid python venv runtime path for model " + model.getModelName() @@ -277,9 +298,9 @@ private void setupModelDependencies(Model model) + pythonRuntime); } - Path requirementsFilePath = - Paths.get(model.getModelDir().getAbsolutePath(), requirementsFile); - List commandParts = new ArrayList<>(); + processBuilder.directory( + Paths.get(EnvironmentUtils.getPythonVenvPath(model)).toFile().getParentFile()); + commandParts.add(pythonRuntime); commandParts.add("-m"); commandParts.add("pip"); @@ -289,54 +310,72 @@ private void setupModelDependencies(Model model) commandParts.add("only-if-needed"); commandParts.add("-r"); commandParts.add(requirementsFilePath.toString()); - - ProcessBuilder processBuilder = new ProcessBuilder(commandParts); - - String[] envp = - EnvironmentUtils.getEnvString( - configManager.getModelServerHome(), - model.getModelDir().getAbsolutePath(), - null); - Map environment = processBuilder.environment(); - for (String envVar : envp) { - String[] parts = envVar.split("=", 2); - if (parts.length == 2) { - environment.put(parts[0], parts[1]); - } + } else { + File dependencyPath = model.getModelDir(); + if (Files.isSymbolicLink(dependencyPath.toPath())) { + dependencyPath = dependencyPath.getParentFile(); + } + if (!isValidDependencyPath(dependencyPath.getPath())) { + throw new ModelException( + "Invalid 3rd party package installation path " + + dependencyPath.getCanonicalPath()); } - processBuilder.directory( - Paths.get(EnvironmentUtils.getPythonVenvPath(model)).toFile().getParentFile()); - processBuilder.redirectErrorStream(true); - Process process = processBuilder.start(); + processBuilder.directory(dependencyPath); - int exitCode = process.waitFor(); - String line; - StringBuilder outputString = new StringBuilder(); - BufferedReader brdr = - new BufferedReader(new InputStreamReader(process.getInputStream())); - while ((line = brdr.readLine()) != null) { - outputString.append(line + "\n"); - } + commandParts.add(pythonRuntime); + commandParts.add("-m"); + commandParts.add("pip"); + commandParts.add("install"); + commandParts.add("-U"); + commandParts.add("-t"); + commandParts.add(dependencyPath.getAbsolutePath()); + commandParts.add("-r"); + commandParts.add(requirementsFilePath.toString()); + } - if (exitCode == 0) { - logger.info( - "Installed custom pip packages for model {}:\n{}", - model.getModelName(), - outputString.toString()); - } else { - logger.error( - "Custom pip package installation failed for model {}:\n{}", - model.getModelName(), - outputString.toString()); - throw new ModelException( - "Custom pip package installation failed for model " + model.getModelName()); + processBuilder.command(commandParts); + String[] envp = + EnvironmentUtils.getEnvString( + configManager.getModelServerHome(), + model.getModelDir().getAbsolutePath(), + null); + Map environment = processBuilder.environment(); + for (String envVar : envp) { + String[] parts = envVar.split("=", 2); + if (parts.length == 2) { + environment.put(parts[0], parts[1]); } } + processBuilder.redirectErrorStream(true); + + Process process = processBuilder.start(); + + int exitCode = process.waitFor(); + String line; + StringBuilder outputString = new StringBuilder(); + BufferedReader brdr = new BufferedReader(new InputStreamReader(process.getInputStream())); + while ((line = brdr.readLine()) != null) { + outputString.append(line + "\n"); + } + + if (exitCode == 0) { + logger.info( + "Installed custom pip packages for model {}:\n{}", + model.getModelName(), + outputString.toString()); + } else { + logger.error( + "Custom pip package installation failed for model {}:\n{}", + model.getModelName(), + outputString.toString()); + throw new ModelException( + "Custom pip package installation failed for model " + model.getModelName()); + } } - private boolean isValidVenvPath(String venvPath) { - if (Paths.get(venvPath) + private boolean isValidDependencyPath(String dependencyPath) { + if (Paths.get(dependencyPath) .normalize() .startsWith(FileUtils.getTempDirectory().toPath().normalize())) { return true; From 5715cf2f9cf59e1fa29b4dadab2b8c530b33298e Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Sun, 4 Feb 2024 22:09:58 -0800 Subject: [PATCH 06/13] Add integration tests and documentation --- .../serve/archive/model/ModelConfig.java | 6 +- model-archiver/README.md | 8 +- .../custom_dependencies/config.properties | 1 + .../mnist_custom_dependencies_handler.py | 20 ++ .../custom_dependencies/model_config.yaml | 1 + .../custom_dependencies/requirements.txt | 1 + test/pytest/test_model_custom_dependencies.py | 293 ++++++++++++++++++ 7 files changed, 324 insertions(+), 6 deletions(-) create mode 100644 test/pytest/test_data/custom_dependencies/config.properties create mode 100644 test/pytest/test_data/custom_dependencies/mnist_custom_dependencies_handler.py create mode 100644 test/pytest/test_data/custom_dependencies/model_config.yaml create mode 100644 test/pytest/test_data/custom_dependencies/requirements.txt create mode 100644 test/pytest/test_model_custom_dependencies.py diff --git a/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java b/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java index 832391141c..b3fe240c32 100644 --- a/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java +++ b/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java @@ -68,9 +68,9 @@ public class ModelConfig { /** continuousBatching is a flag to enable continuous batching. */ private boolean continuousBatching; /** - * Create python virtual environment when using python backend to: 1) Install model dependencies - * (if enabled globally using install_py_dep_per_model=true) 2) Run workers for model loading - * and inference + * Create python virtual environment when using python backend to install model dependencies (if + * enabled globally using install_py_dep_per_model=true) and run workers for model loading and + * inference. */ private boolean useVenv; diff --git a/model-archiver/README.md b/model-archiver/README.md index ae9df6f880..b7c8d7be3a 100644 --- a/model-archiver/README.md +++ b/model-archiver/README.md @@ -161,19 +161,21 @@ For more details refer [default handler documentation](../docs/default_handlers. ### Config file -A model config yaml file. For example: +A model config yaml file. For example: ``` # TS frontend parameters -# See all supported parameters: https://github.com/pytorch/serve/blob/master/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java#L14 +# See all supported parameters: https://github.com/pytorch/serve/blob/master/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java#L14 minWorkers: 1 # default: #CPU or #GPU maxWorkers: 1 # default: #CPU or #GPU batchSize: 1 # default: 1 maxBatchDelay: 100 # default: 100 msec responseTimeout: 120 # default: 120 sec deviceType: cpu # cpu, gpu, neuron -deviceIds: [0,1,2,3] # gpu device ids allocated to this model. +deviceIds: [0,1,2,3] # gpu device ids allocated to this model. parallelType: pp # pp: pipeline parallel; pptp: tensor+pipeline parallel. Default: empty +useVenv: Create python virtual environment when using python backend to install model dependencies (if enabled globally using install_py_dep_per_model=true) + and run workers for model loading and inference. # See torchrun parameters: https://pytorch.org/docs/stable/elastic/run.html torchrun: diff --git a/test/pytest/test_data/custom_dependencies/config.properties b/test/pytest/test_data/custom_dependencies/config.properties new file mode 100644 index 0000000000..8a0c798580 --- /dev/null +++ b/test/pytest/test_data/custom_dependencies/config.properties @@ -0,0 +1 @@ +install_py_dep_per_model=true diff --git a/test/pytest/test_data/custom_dependencies/mnist_custom_dependencies_handler.py b/test/pytest/test_data/custom_dependencies/mnist_custom_dependencies_handler.py new file mode 100644 index 0000000000..4c203e7b78 --- /dev/null +++ b/test/pytest/test_data/custom_dependencies/mnist_custom_dependencies_handler.py @@ -0,0 +1,20 @@ +# import custom dependency to test that it has been installed +import matplotlib.pyplot as pyplt +from torchvision import transforms + +from ts.torch_handler.image_classifier import ImageClassifier + + +class MNISTDigitClassifier(ImageClassifier): + image_processing = transforms.Compose( + [transforms.ToTensor(), transforms.Normalize((0.1307,), (0.3081,))] + ) + + def __init__(self): + super(MNISTDigitClassifier, self).__init__() + + def postprocess(self, data): + result = data.argmax(1).tolist() + # test that custom dependency works + pyplt.plot(result) + return result diff --git a/test/pytest/test_data/custom_dependencies/model_config.yaml b/test/pytest/test_data/custom_dependencies/model_config.yaml new file mode 100644 index 0000000000..14224bbdc9 --- /dev/null +++ b/test/pytest/test_data/custom_dependencies/model_config.yaml @@ -0,0 +1 @@ +useVenv: true diff --git a/test/pytest/test_data/custom_dependencies/requirements.txt b/test/pytest/test_data/custom_dependencies/requirements.txt new file mode 100644 index 0000000000..6ccafc3f90 --- /dev/null +++ b/test/pytest/test_data/custom_dependencies/requirements.txt @@ -0,0 +1 @@ +matplotlib diff --git a/test/pytest/test_model_custom_dependencies.py b/test/pytest/test_model_custom_dependencies.py new file mode 100644 index 0000000000..17d14cf37c --- /dev/null +++ b/test/pytest/test_model_custom_dependencies.py @@ -0,0 +1,293 @@ +import os +import subprocess + +import requests +import test_utils + + +def setup_module(module): + test_utils.torchserve_cleanup() + # Clean out custom model dependencies in base python environment + subprocess.run( + [ + "pip", + "uninstall", + "-y", + "-r", + str( + os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "requirements.txt", + ) + ), + ], + check=True, + ) + + +def teardown_module(module): + test_utils.torchserve_cleanup() + + +def generate_model_archive(use_requirements=False, use_venv=False): + model_archiver_cmd = test_utils.model_archiver_command_builder( + model_name="mnist_custom_dependencies", + version="1.0", + model_file=os.path.join( + test_utils.REPO_ROOT, "examples", "image_classifier", "mnist", "mnist.py" + ), + serialized_file=os.path.join( + test_utils.REPO_ROOT, + "examples", + "image_classifier", + "mnist", + "mnist_cnn.pt", + ), + handler=os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "mnist_custom_dependencies_handler.py", + ), + requirements_file=os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "requirements.txt", + ) + if use_requirements + else None, + config_file=os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "model_config.yaml", + ) + if use_venv + else None, + export_path=test_utils.MODEL_STORE, + force=True, + ) + model_archiver_cmd = model_archiver_cmd.split(" ") + subprocess.run(model_archiver_cmd, check=True) + + +def register_model_and_make_inference_request(expect_model_load_failure=False): + try: + resp = test_utils.register_model( + "mnist_custom_dependencies", "mnist_custom_dependencies.mar" + ) + resp.raise_for_status() + except Exception as e: + if expect_model_load_failure: + return + else: + raise e + + if expect_model_load_failure: + raise Exception("Expected model load failure but model load succeeded") + + data_file = os.path.join( + test_utils.REPO_ROOT, + "examples", + "image_classifier", + "mnist", + "test_data", + "0.png", + ) + with open(data_file, "rb") as input_data: + resp = requests.post( + url=f"http://localhost:8080/predictions/mnist_custom_dependencies", + data=input_data, + ) + resp.raise_for_status() + + +def test_install_dependencies_to_target_directory_with_requirements(): + # Torchserve cleanup + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + try: + generate_model_archive(use_requirements=True, use_venv=False) + test_utils.start_torchserve( + model_store=test_utils.MODEL_STORE, + snapshot_file=os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "config.properties", + ), + no_config_snapshots=True, + gen_mar=False, + ) + register_model_and_make_inference_request(expect_model_load_failure=False) + finally: + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + +def test_install_dependencies_to_target_directory_without_requirements(): + # Torchserve cleanup + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + try: + generate_model_archive(use_requirements=False, use_venv=False) + test_utils.start_torchserve( + model_store=test_utils.MODEL_STORE, + snapshot_file=os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "config.properties", + ), + no_config_snapshots=True, + gen_mar=False, + ) + register_model_and_make_inference_request(expect_model_load_failure=True) + finally: + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + +def test_disable_install_dependencies_to_target_directory_with_requirements(): + # Torchserve cleanup + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + try: + generate_model_archive(use_requirements=True, use_venv=False) + test_utils.start_torchserve( + model_store=test_utils.MODEL_STORE, + snapshot_file=None, + no_config_snapshots=True, + gen_mar=False, + ) + register_model_and_make_inference_request(expect_model_load_failure=True) + finally: + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + +def test_disable_install_dependencies_to_target_directory_without_requirements(): + # Torchserve cleanup + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + try: + generate_model_archive(use_requirements=False, use_venv=False) + test_utils.start_torchserve( + model_store=test_utils.MODEL_STORE, + snapshot_file=None, + no_config_snapshots=True, + gen_mar=False, + ) + register_model_and_make_inference_request(expect_model_load_failure=True) + finally: + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + +def test_install_dependencies_to_venv_with_requirements(): + # Torchserve cleanup + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + try: + generate_model_archive(use_requirements=True, use_venv=True) + test_utils.start_torchserve( + model_store=test_utils.MODEL_STORE, + snapshot_file=os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "config.properties", + ), + no_config_snapshots=True, + gen_mar=False, + ) + register_model_and_make_inference_request(expect_model_load_failure=False) + finally: + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + +def test_install_dependencies_to_venv_without_requirements(): + # Torchserve cleanup + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + try: + generate_model_archive(use_requirements=False, use_venv=True) + test_utils.start_torchserve( + model_store=test_utils.MODEL_STORE, + snapshot_file=os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "config.properties", + ), + no_config_snapshots=True, + gen_mar=False, + ) + register_model_and_make_inference_request(expect_model_load_failure=True) + finally: + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + +def test_disable_install_dependencies_to_venv_with_requirements(): + # Torchserve cleanup + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + try: + generate_model_archive(use_requirements=True, use_venv=True) + test_utils.start_torchserve( + model_store=test_utils.MODEL_STORE, + snapshot_file=None, + no_config_snapshots=True, + gen_mar=False, + ) + register_model_and_make_inference_request(expect_model_load_failure=True) + finally: + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + +def test_disable_install_dependencies_to_venv_without_requirements(): + # Torchserve cleanup + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() + + try: + generate_model_archive(use_requirements=False, use_venv=True) + test_utils.start_torchserve( + model_store=test_utils.MODEL_STORE, + snapshot_file=None, + no_config_snapshots=True, + gen_mar=False, + ) + register_model_and_make_inference_request(expect_model_load_failure=True) + finally: + test_utils.stop_torchserve() + test_utils.delete_all_snapshots() From 6c4a279c1dd730acc7e797b4d4df993962acdbc8 Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Mon, 5 Feb 2024 12:00:56 -0800 Subject: [PATCH 07/13] Fix integraiton test failures --- test/pytest/test_model_custom_dependencies.py | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/test/pytest/test_model_custom_dependencies.py b/test/pytest/test_model_custom_dependencies.py index 17d14cf37c..e6cade5381 100644 --- a/test/pytest/test_model_custom_dependencies.py +++ b/test/pytest/test_model_custom_dependencies.py @@ -7,26 +7,6 @@ def setup_module(module): test_utils.torchserve_cleanup() - # Clean out custom model dependencies in base python environment - subprocess.run( - [ - "pip", - "uninstall", - "-y", - "-r", - str( - os.path.join( - test_utils.REPO_ROOT, - "test", - "pytest", - "test_data", - "custom_dependencies", - "requirements.txt", - ) - ), - ], - check=True, - ) def teardown_module(module): From 5101d6b578da461cc33d7164331f799baeaf7f32 Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Mon, 5 Feb 2024 12:16:26 -0800 Subject: [PATCH 08/13] Revert "Fix integraiton test failures" This reverts commit 6c4a279c1dd730acc7e797b4d4df993962acdbc8. --- test/pytest/test_model_custom_dependencies.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/pytest/test_model_custom_dependencies.py b/test/pytest/test_model_custom_dependencies.py index e6cade5381..17d14cf37c 100644 --- a/test/pytest/test_model_custom_dependencies.py +++ b/test/pytest/test_model_custom_dependencies.py @@ -7,6 +7,26 @@ def setup_module(module): test_utils.torchserve_cleanup() + # Clean out custom model dependencies in base python environment + subprocess.run( + [ + "pip", + "uninstall", + "-y", + "-r", + str( + os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "requirements.txt", + ) + ), + ], + check=True, + ) def teardown_module(module): From 62a82cffb08092aacbe9af713100288794d2a13e Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Mon, 5 Feb 2024 12:33:04 -0800 Subject: [PATCH 09/13] Update integration test teardown functionality --- test/pytest/test_model_custom_dependencies.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/pytest/test_model_custom_dependencies.py b/test/pytest/test_model_custom_dependencies.py index 17d14cf37c..ef14517414 100644 --- a/test/pytest/test_model_custom_dependencies.py +++ b/test/pytest/test_model_custom_dependencies.py @@ -31,6 +31,25 @@ def setup_module(module): def teardown_module(module): test_utils.torchserve_cleanup() + # Restore custom model dependencies in base python environment + subprocess.run( + [ + "pip", + "install", + "-r", + str( + os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "requirements.txt", + ) + ), + ], + check=True, + ) def generate_model_archive(use_requirements=False, use_venv=False): From 750f2e05c50b6766b4af2170f489d940fb873fa6 Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Wed, 7 Feb 2024 15:23:53 -0800 Subject: [PATCH 10/13] Refactor implementaiton to check for useVenv in Model.java --- .../serve/util/messages/EnvironmentUtils.java | 17 +++---- .../java/org/pytorch/serve/wlm/Model.java | 6 +++ .../org/pytorch/serve/wlm/ModelManager.java | 46 +++++++++---------- 3 files changed, 36 insertions(+), 33 deletions(-) diff --git a/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java b/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java index 43708c0bc3..3b5a6779b2 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java +++ b/frontend/server/src/main/java/org/pytorch/serve/util/messages/EnvironmentUtils.java @@ -10,7 +10,6 @@ import java.util.Map; import java.util.regex.Pattern; import org.pytorch.serve.archive.model.Manifest; -import org.pytorch.serve.archive.model.ModelConfig; import org.pytorch.serve.util.ConfigManager; import org.pytorch.serve.wlm.Model; import org.slf4j.Logger; @@ -77,14 +76,12 @@ public static String[] getEnvString(String cwd, String modelPath, String handler public static String getPythonRunTime(Model model) { String pythonRuntime; - Manifest.RuntimeType runtime = model.getModelArchive().getManifest().getRuntime(); + Manifest.RuntimeType runtime = model.getRuntimeType(); if (runtime == Manifest.RuntimeType.PYTHON) { pythonRuntime = configManager.getPythonExecutable(); - ModelConfig modelConfig = model.getModelArchive().getModelConfig(); - Path pythonVenvRuntime = Paths.get(getPythonVenvPath(model), "bin", "python"); - if (modelConfig != null - && modelConfig.getUseVenv() == true - && Files.exists(pythonVenvRuntime)) { + Path pythonVenvRuntime = + Paths.get(getPythonVenvPath(model).toString(), "bin", "python"); + if (model.isUseVenv() && Files.exists(pythonVenvRuntime)) { pythonRuntime = pythonVenvRuntime.toString(); } } else { @@ -93,13 +90,13 @@ public static String getPythonRunTime(Model model) { return pythonRuntime; } - public static String getPythonVenvPath(Model model) { + public static File getPythonVenvPath(Model model) { File modelDir = model.getModelDir(); if (Files.isSymbolicLink(modelDir.toPath())) { modelDir = modelDir.getParentFile(); } - Path venvPath = Paths.get(modelDir.getAbsolutePath(), "venv"); - return venvPath.toString(); + Path venvPath = Paths.get(modelDir.getAbsolutePath(), "venv").toAbsolutePath(); + return venvPath.toFile(); } public static String[] getCppEnvString(String libPath) { diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/Model.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/Model.java index cfe0de74dc..c478104d3e 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/Model.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/Model.java @@ -82,11 +82,13 @@ public class Model { private boolean useJobTicket; private AtomicInteger numJobTickets; private boolean continuousBatching; + private boolean useVenv; public Model(ModelArchive modelArchive, int queueSize) { this.modelArchive = modelArchive; if (modelArchive != null && modelArchive.getModelConfig() != null) { continuousBatching = modelArchive.getModelConfig().isContinuousBatching(); + useVenv = modelArchive.getModelConfig().getUseVenv(); if (modelArchive.getModelConfig().getParallelLevel() > 0 && modelArchive.getModelConfig().getParallelType() != ModelConfig.ParallelType.NONE) { @@ -630,6 +632,10 @@ public boolean isContinuousBatching() { return continuousBatching; } + public boolean isUseVenv() { + return useVenv; + } + public boolean hasTensorParallel() { switch (this.parallelType) { case PP: diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java index 9c8afaeb52..7830ffae66 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java @@ -211,29 +211,29 @@ private ModelArchive createModelArchive( private void setupModelVenv(Model model) throws IOException, InterruptedException, ModelException { - ModelConfig modelConfig = model.getModelArchive().getModelConfig(); - if (model.getModelArchive().getManifest().getRuntime() != Manifest.RuntimeType.PYTHON - || modelConfig == null - || modelConfig.getUseVenv() != true) { + if (model.getRuntimeType() != Manifest.RuntimeType.PYTHON || !model.isUseVenv()) { return; } - String venvPath = EnvironmentUtils.getPythonVenvPath(model); + File venvPath = EnvironmentUtils.getPythonVenvPath(model); List commandParts = new ArrayList<>(); commandParts.add(configManager.getPythonExecutable()); commandParts.add("-m"); commandParts.add("venv"); commandParts.add("--clear"); commandParts.add("--system-site-packages"); - commandParts.add(venvPath); + commandParts.add(venvPath.toString()); ProcessBuilder processBuilder = new ProcessBuilder(commandParts); if (isValidDependencyPath(venvPath)) { - processBuilder.directory(Paths.get(venvPath).toFile().getParentFile()); + processBuilder.directory(venvPath.getParentFile()); } else { throw new ModelException( - "Invalid python venv path for model " + model.getModelName() + ": " + venvPath); + "Invalid python venv path for model " + + model.getModelName() + + ": " + + venvPath.toString()); } Map environment = processBuilder.environment(); String[] envp = @@ -261,12 +261,14 @@ private void setupModelVenv(Model model) if (exitCode == 0) { logger.info( - "Created virtual environment for model {}: {}", model.getModelName(), venvPath); + "Created virtual environment for model {}: {}", + model.getModelName(), + venvPath.toString()); } else { logger.error( "Virtual environment creation for model {} at {} failed:\n{}", model.getModelName(), - venvPath, + venvPath.toString(), outputString.toString()); throw new ModelException( "Virtual environment creation failed for model " + model.getModelName()); @@ -282,15 +284,14 @@ private void setupModelDependencies(Model model) return; } - ModelConfig modelConfig = model.getModelArchive().getModelConfig(); String pythonRuntime = EnvironmentUtils.getPythonRunTime(model); Path requirementsFilePath = - Paths.get(model.getModelDir().getAbsolutePath(), requirementsFile); + Paths.get(model.getModelDir().getAbsolutePath(), requirementsFile).toAbsolutePath(); List commandParts = new ArrayList<>(); ProcessBuilder processBuilder = new ProcessBuilder(); - if (modelConfig != null && modelConfig.getUseVenv() == true) { - if (!isValidDependencyPath(pythonRuntime)) { + if (model.isUseVenv()) { + if (!isValidDependencyPath(Paths.get(pythonRuntime).toFile())) { throw new ModelException( "Invalid python venv runtime path for model " + model.getModelName() @@ -298,8 +299,7 @@ private void setupModelDependencies(Model model) + pythonRuntime); } - processBuilder.directory( - Paths.get(EnvironmentUtils.getPythonVenvPath(model)).toFile().getParentFile()); + processBuilder.directory(EnvironmentUtils.getPythonVenvPath(model).getParentFile()); commandParts.add(pythonRuntime); commandParts.add("-m"); @@ -311,14 +311,13 @@ private void setupModelDependencies(Model model) commandParts.add("-r"); commandParts.add(requirementsFilePath.toString()); } else { - File dependencyPath = model.getModelDir(); + File dependencyPath = model.getModelDir().getAbsolutePath(); if (Files.isSymbolicLink(dependencyPath.toPath())) { dependencyPath = dependencyPath.getParentFile(); } - if (!isValidDependencyPath(dependencyPath.getPath())) { + if (!isValidDependencyPath(dependencyPath)) { throw new ModelException( - "Invalid 3rd party package installation path " - + dependencyPath.getCanonicalPath()); + "Invalid 3rd party package installation path " + dependencyPath.toString()); } processBuilder.directory(dependencyPath); @@ -329,7 +328,7 @@ private void setupModelDependencies(Model model) commandParts.add("install"); commandParts.add("-U"); commandParts.add("-t"); - commandParts.add(dependencyPath.getAbsolutePath()); + commandParts.add(dependencyPath.toString()); commandParts.add("-r"); commandParts.add(requirementsFilePath.toString()); } @@ -374,8 +373,9 @@ private void setupModelDependencies(Model model) } } - private boolean isValidDependencyPath(String dependencyPath) { - if (Paths.get(dependencyPath) + private boolean isValidDependencyPath(File dependencyPath) { + if (dependencyPath + .toPath() .normalize() .startsWith(FileUtils.getTempDirectory().toPath().normalize())) { return true; From 760fac962ca2d0659d77c689efe5e45a3f5813ef Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Wed, 7 Feb 2024 15:38:23 -0800 Subject: [PATCH 11/13] Fix dependencyPath logic --- .../src/main/java/org/pytorch/serve/wlm/ModelManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java index 7830ffae66..a37f5e46c6 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java @@ -311,9 +311,9 @@ private void setupModelDependencies(Model model) commandParts.add("-r"); commandParts.add(requirementsFilePath.toString()); } else { - File dependencyPath = model.getModelDir().getAbsolutePath(); + File dependencyPath = model.getModelDir().getAbsoluteFile(); if (Files.isSymbolicLink(dependencyPath.toPath())) { - dependencyPath = dependencyPath.getParentFile(); + dependencyPath = dependencyPath.getParentFile().getAbsoluteFile(); } if (!isValidDependencyPath(dependencyPath)) { throw new ModelException( From 603f440b6f4860a76ae6b19d786db420ee523ac3 Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Wed, 7 Feb 2024 17:57:03 -0800 Subject: [PATCH 12/13] Refactor isUseVenv and integration tests --- .../serve/archive/model/ModelConfig.java | 4 +- .../java/org/pytorch/serve/wlm/Model.java | 6 ++- .../org/pytorch/serve/wlm/ModelManager.java | 7 ++-- test/pytest/test_model_custom_dependencies.py | 38 +++++++++++-------- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java b/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java index b3fe240c32..fd67561c0e 100644 --- a/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java +++ b/frontend/archive/src/main/java/org/pytorch/serve/archive/model/ModelConfig.java @@ -69,8 +69,8 @@ public class ModelConfig { private boolean continuousBatching; /** * Create python virtual environment when using python backend to install model dependencies (if - * enabled globally using install_py_dep_per_model=true) and run workers for model loading and - * inference. + * enabled globally using configuration install_py_dep_per_model=true) and run workers for model + * loading and inference. */ private boolean useVenv; diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/Model.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/Model.java index c478104d3e..74ea1ba869 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/Model.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/Model.java @@ -633,7 +633,11 @@ public boolean isContinuousBatching() { } public boolean isUseVenv() { - return useVenv; + if (getRuntimeType() == Manifest.RuntimeType.PYTHON) { + return useVenv; + } else { + return false; + } } public boolean hasTensorParallel() { diff --git a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java index a37f5e46c6..7e0c64e496 100644 --- a/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java +++ b/frontend/server/src/main/java/org/pytorch/serve/wlm/ModelManager.java @@ -211,7 +211,7 @@ private ModelArchive createModelArchive( private void setupModelVenv(Model model) throws IOException, InterruptedException, ModelException { - if (model.getRuntimeType() != Manifest.RuntimeType.PYTHON || !model.isUseVenv()) { + if (!model.isUseVenv()) { return; } @@ -311,10 +311,11 @@ private void setupModelDependencies(Model model) commandParts.add("-r"); commandParts.add(requirementsFilePath.toString()); } else { - File dependencyPath = model.getModelDir().getAbsoluteFile(); + File dependencyPath = model.getModelDir(); if (Files.isSymbolicLink(dependencyPath.toPath())) { - dependencyPath = dependencyPath.getParentFile().getAbsoluteFile(); + dependencyPath = dependencyPath.getParentFile(); } + dependencyPath = dependencyPath.getAbsoluteFile(); if (!isValidDependencyPath(dependencyPath)) { throw new ModelException( "Invalid 3rd party package installation path " + dependencyPath.toString()); diff --git a/test/pytest/test_model_custom_dependencies.py b/test/pytest/test_model_custom_dependencies.py index ef14517414..4a8fec5151 100644 --- a/test/pytest/test_model_custom_dependencies.py +++ b/test/pytest/test_model_custom_dependencies.py @@ -1,8 +1,11 @@ import os +import pathlib import subprocess import requests import test_utils +from model_archiver import ModelArchiver, ModelArchiverConfig +from model_archiver.manifest_components.manifest import RuntimeType def setup_module(module): @@ -27,6 +30,8 @@ def setup_module(module): ], check=True, ) + # Create model store directory + pathlib.Path(test_utils.MODEL_STORE).mkdir(parents=True, exist_ok=True) def teardown_module(module): @@ -53,9 +58,17 @@ def teardown_module(module): def generate_model_archive(use_requirements=False, use_venv=False): - model_archiver_cmd = test_utils.model_archiver_command_builder( + config = ModelArchiverConfig( model_name="mnist_custom_dependencies", - version="1.0", + handler=os.path.join( + test_utils.REPO_ROOT, + "test", + "pytest", + "test_data", + "custom_dependencies", + "mnist_custom_dependencies_handler.py", + ), + runtime=RuntimeType.PYTHON.value, model_file=os.path.join( test_utils.REPO_ROOT, "examples", "image_classifier", "mnist", "mnist.py" ), @@ -66,14 +79,11 @@ def generate_model_archive(use_requirements=False, use_venv=False): "mnist", "mnist_cnn.pt", ), - handler=os.path.join( - test_utils.REPO_ROOT, - "test", - "pytest", - "test_data", - "custom_dependencies", - "mnist_custom_dependencies_handler.py", - ), + extra_files=None, + export_path=test_utils.MODEL_STORE, + force=True, + archive_format="no-archive", + version="1.0", requirements_file=os.path.join( test_utils.REPO_ROOT, "test", @@ -94,17 +104,15 @@ def generate_model_archive(use_requirements=False, use_venv=False): ) if use_venv else None, - export_path=test_utils.MODEL_STORE, - force=True, ) - model_archiver_cmd = model_archiver_cmd.split(" ") - subprocess.run(model_archiver_cmd, check=True) + + ModelArchiver.generate_model_archive(config) def register_model_and_make_inference_request(expect_model_load_failure=False): try: resp = test_utils.register_model( - "mnist_custom_dependencies", "mnist_custom_dependencies.mar" + "mnist_custom_dependencies", "mnist_custom_dependencies" ) resp.raise_for_status() except Exception as e: From 4f470f5c42338ffa40191465cd2cf8a3e2228f9e Mon Sep 17 00:00:00 2001 From: Naman Nandan Date: Wed, 7 Feb 2024 18:18:42 -0800 Subject: [PATCH 13/13] Update documentation --- model-archiver/README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/model-archiver/README.md b/model-archiver/README.md index b7c8d7be3a..ab18ba3ba9 100644 --- a/model-archiver/README.md +++ b/model-archiver/README.md @@ -174,8 +174,13 @@ responseTimeout: 120 # default: 120 sec deviceType: cpu # cpu, gpu, neuron deviceIds: [0,1,2,3] # gpu device ids allocated to this model. parallelType: pp # pp: pipeline parallel; pptp: tensor+pipeline parallel. Default: empty -useVenv: Create python virtual environment when using python backend to install model dependencies (if enabled globally using install_py_dep_per_model=true) - and run workers for model loading and inference. +useVenv: Create python virtual environment when using python backend to install model dependencies + (if enabled globally using install_py_dep_per_model=true) and run workers for model loading + and inference. Note that, although creation of virtual environment adds a latency overhead + (approx. 2 to 3 seconds) during model load and disk space overhead (approx. 25M), overall + it can speed up load time and reduce disk utilization for models with custom dependencies + since it enables reusing custom packages(specified in requirements.txt) and their + supported dependencies that are already available in the base python environment. # See torchrun parameters: https://pytorch.org/docs/stable/elastic/run.html torchrun: