Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix GRPC address assignment to localhost by default #3083

Merged
merged 4 commits into from
Apr 11, 2024
Merged

Fix GRPC address assignment to localhost by default #3083

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
330a7f6
Fix GRPC address assignment to localhost by default
namannandan Apr 11, 2024
3e02620
Update documentation
namannandan Apr 11, 2024
d96dd32
Merge branch 'master' into fix-grpc-address
namannandan Apr 11, 2024
a6ee72e
fix typos
namannandan Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docker/config.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
inference_address=http://0.0.0.0:8080
management_address=http://0.0.0.0:8081
metrics_address=http://0.0.0.0:8082
grpc_inference_address=0.0.0.0
Copy link
Member

@msaroufim msaroufim Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is the port set now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It uses the default values from here:

serve/frontend/server/src/main/java/org/pytorch/serve/util/ConfigManager.java

Lines 360 to 368 in 89c5389

public int getGRPCPort(ConnectorType connectorType) {
String port;
if (connectorType == ConnectorType.MANAGEMENT_CONNECTOR) {
port = prop.getProperty(TS_GRPC_MANAGEMENT_PORT, "7071");
} else {
port = prop.getProperty(TS_GRPC_INFERENCE_PORT, "7070");
}
return Integer.parseInt(port);
}

And is called here: https://github.com/pytorch/serve/pull/3083/files#diff-45f9e03a9d3046ef0e2c18e18b0bb9c1889f9e3ff9918568870539cc926e3895R454

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added configuration option to set grpc address separately from port to maintain backwards compatibility to retain the grpc port configuration:

serve/frontend/server/src/main/java/org/pytorch/serve/util/ConfigManager.java

Lines 104 to 105 in 89c5389

private static final String TS_GRPC_INFERENCE_PORT = "grpc_inference_port";
private static final String TS_GRPC_MANAGEMENT_PORT = "grpc_management_port";

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep this consistent?
grpc_inference_address=http://0.0.0.0:7070
grpc_management_address=http://0.0.0.0:7071

Copy link
Collaborator Author

@namannandan namannandan Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to support configuration format as grpc_inference_address=http://0.0.0.0:7070 but we've had support to configure the GRPC ports alone since v0.3.0:

serve/frontend/server/src/main/java/org/pytorch/serve/util/ConfigManager.java

Lines 104 to 105 in 89c5389

private static final String TS_GRPC_INFERENCE_PORT = "grpc_inference_port";
private static final String TS_GRPC_MANAGEMENT_PORT = "grpc_management_port";

To maintain backwards compatibility, I believe we'll need to retain the grpc port configuration.

If we were to allow specifying the port along with the address as grpc_inference_address=http://0.0.0.0:7070, I believe we'll have two options:

  1. Ignore the grpc port configuration ex grpc_inference_port if grpc_inference_address includes the port already.
  2. Override the port specified in grpc_inference_address with the port specified in grpc_inference_port.

Another option is to potentially rename grpc_inference_address to grpc_inference_ip_address to the make the configuration name more explicit to convey what the value is intended to be.

Let me know your thoughts or suggestions on better ways to handle it.

grpc_management_address=0.0.0.0
number_of_netty_threads=32
job_queue_size=1000
model_store=/home/model-server/model-store
Expand Down
9 changes: 7 additions & 2 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,13 @@ inference_address=https://127.0.0.1:8443
inference_address=https://172.16.1.10:8080
```

### Configure TorchServe gRPC listening ports
The inference gRPC API is listening on port 7070, and the management gRPC API is listening on port 7071 by default.
### Configure TorchServe gRPC listening addresses and ports
The inference gRPC API is listening on port 7070, and the management gRPC API is listening on port 7071 on localhost by default.

To configure different addresses use following properties

* `grpc_inference_address`: Inference gRPC API IP address. Default: 127.0.0.1
* `grpc_management_address`: Management gRPC API IP address. Default: 127.0.0.1

To configure different ports use following properties

Expand Down
4 changes: 2 additions & 2 deletions docs/grpc_api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ TorchServe provides following gRPCs apis
- **DescribeModel** : Get detail runtime status of default version of a model
- **SetDefault** : Set any registered version of a model as default version

By default, TorchServe listens on port 7070 for the gRPC Inference API and 7071 for the gRPC Management API.
To configure gRPC APIs on different ports refer [configuration documentation](configuration.md)
By default, TorchServe listens on port 7070 for the gRPC Inference API and 7071 for the gRPC Management API on localhost.
To configure gRPC APIs on different address and ports refer [configuration documentation](configuration.md)

## Python client example for gRPC APIs

Expand Down
6 changes: 5 additions & 1 deletion frontend/server/src/main/java/org/pytorch/serve/ModelServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.io.File;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.net.InetSocketAddress;
import java.security.GeneralSecurityException;
import java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -447,7 +448,10 @@ public void startGRPCServers() throws IOException {
private Server startGRPCServer(ConnectorType connectorType) throws IOException {

ServerBuilder<?> s =
NettyServerBuilder.forPort(configManager.getGRPCPort(connectorType))
NettyServerBuilder.forAddress(
new InetSocketAddress(
configManager.getGRPCAddress(connectorType),
configManager.getGRPCPort(connectorType)))
.maxInboundMessageSize(configManager.getMaxRequestSize())
.addService(
ServerInterceptors.intercept(
Expand Down
21 changes: 19 additions & 2 deletions frontend/server/src/main/java/org/pytorch/serve/util/ConfigManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ public final class ConfigManager {
private static final String TS_ALLOWED_URLS = "allowed_urls";
private static final String TS_INSTALL_PY_DEP_PER_MODEL = "install_py_dep_per_model";
private static final String TS_ENABLE_METRICS_API = "enable_metrics_api";
private static final String TS_GRPC_INFERENCE_ADDRESS = "grpc_inference_address";
private static final String TS_GRPC_MANAGEMENT_ADDRESS = "grpc_management_address";
private static final String TS_GRPC_INFERENCE_PORT = "grpc_inference_port";
private static final String TS_GRPC_MANAGEMENT_PORT = "grpc_management_port";
private static final String TS_ENABLE_GRPC_SSL = "enable_grpc_ssl";
Expand Down Expand Up @@ -357,12 +359,27 @@ public Connector getListener(ConnectorType connectorType) {
return Connector.parse(binding, connectorType);
}

public int getGRPCPort(ConnectorType connectorType) {
public InetAddress getGRPCAddress(ConnectorType connectorType)
throws UnknownHostException, IllegalArgumentException {
if (connectorType == ConnectorType.MANAGEMENT_CONNECTOR) {
return InetAddress.getByName(prop.getProperty(TS_GRPC_MANAGEMENT_ADDRESS, "127.0.0.1"));
} else if (connectorType == ConnectorType.INFERENCE_CONNECTOR) {
return InetAddress.getByName(prop.getProperty(TS_GRPC_INFERENCE_ADDRESS, "127.0.0.1"));
} else {
throw new IllegalArgumentException(
"Connector type not supporte dy gRPC: " + connectorType);
}
}

public int getGRPCPort(ConnectorType connectorType) throws IllegalArgumentException {
String port;
if (connectorType == ConnectorType.MANAGEMENT_CONNECTOR) {
port = prop.getProperty(TS_GRPC_MANAGEMENT_PORT, "7071");
} else {
} else if (connectorType == ConnectorType.INFERENCE_CONNECTOR) {
port = prop.getProperty(TS_GRPC_INFERENCE_PORT, "7070");
} else {
throw new IllegalArgumentException(
"Connector type not supporte dy gRPC: " + connectorType);
}
return Integer.parseInt(port);
}
Expand Down
Loading