ibzib commented on a change in pull request #14942:
URL: https://github.com/apache/beam/pull/14942#discussion_r646952453
##########
File path:
runners/portability/java/src/main/java/org/apache/beam/runners/portability/ExternalWorkerService.java
##########
@@ -90,10 +97,55 @@ public void stopWorker(
public void close() {}
public GrpcFnServer<ExternalWorkerService> start() throws Exception {
- GrpcFnServer<ExternalWorkerService> server =
- GrpcFnServer.allocatePortAndCreateFor(this, serverFactory);
+ final String externalServiceAddress =
+
Environments.getExternalServiceAddress(options.as(PortablePipelineOptions.class));
+ GrpcFnServer<ExternalWorkerService> server;
+ if (externalServiceAddress.isEmpty()) {
+ server = GrpcFnServer.allocatePortAndCreateFor(this, serverFactory);
+ } else {
+ server =
+ GrpcFnServer.create(
+ this,
+
Endpoints.ApiServiceDescriptor.newBuilder().setUrl(externalServiceAddress).build(),
+ serverFactory);
+ }
LOG.debug(
"Listening for worker start requests at {}.",
server.getApiServiceDescriptor().getUrl());
return server;
}
+
+ /**
+ * Worker pool entry point.
+ *
+ * <p>The worker pool exposes an RPC service that is used with EXTERNAL
environment to start and
+ * stop the SDK workers.
+ *
+ * <p>The worker pool uses threads for parallelism;
+ *
+ * <p>This entry point is used by the Java SDK container in worker pool mode.
+ */
+ public static void main(String[] args) throws Exception {
+ main(System::getenv);
+ }
+
+ public static void main(Function<String, String> environmentVarGetter)
throws Exception {
+ System.out.format("Starting external worker service%n");
Review comment:
I'm not sure why `FnHarness#main` uses System.out. It looks like
`FnHarness#main` expects the logger output to be intercepted by
BeamFnLoggingClient, but I don't see why this precludes the use of the logger
before BeamFnLoggingClient is started.
https://github.com/apache/beam/blob/3f2351f02b2d7e981a116ac614b53d170cef2236/sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java#L221-L222
Let's go with the logger unless we find a compelling reason not to.
##########
File path:
runners/portability/java/src/main/java/org/apache/beam/runners/portability/ExternalWorkerService.java
##########
@@ -90,10 +97,55 @@ public void stopWorker(
public void close() {}
public GrpcFnServer<ExternalWorkerService> start() throws Exception {
- GrpcFnServer<ExternalWorkerService> server =
- GrpcFnServer.allocatePortAndCreateFor(this, serverFactory);
+ final String externalServiceAddress =
+
Environments.getExternalServiceAddress(options.as(PortablePipelineOptions.class));
+ GrpcFnServer<ExternalWorkerService> server;
+ if (externalServiceAddress.isEmpty()) {
+ server = GrpcFnServer.allocatePortAndCreateFor(this, serverFactory);
+ } else {
+ server =
+ GrpcFnServer.create(
+ this,
+
Endpoints.ApiServiceDescriptor.newBuilder().setUrl(externalServiceAddress).build(),
+ serverFactory);
+ }
LOG.debug(
"Listening for worker start requests at {}.",
server.getApiServiceDescriptor().getUrl());
return server;
}
+
+ /**
+ * Worker pool entry point.
+ *
+ * <p>The worker pool exposes an RPC service that is used with EXTERNAL
environment to start and
+ * stop the SDK workers.
+ *
+ * <p>The worker pool uses threads for parallelism;
+ *
+ * <p>This entry point is used by the Java SDK container in worker pool mode.
+ */
+ public static void main(String[] args) throws Exception {
+ main(System::getenv);
+ }
+
+ public static void main(Function<String, String> environmentVarGetter)
throws Exception {
+ System.out.format("Starting external worker service%n");
+ System.out.format("Pipeline options %s%n",
environmentVarGetter.apply(PIPELINE_OPTIONS));
+ PipelineOptions options =
+
PipelineOptionsTranslation.fromJson(environmentVarGetter.apply(PIPELINE_OPTIONS));
+
+ try (GrpcFnServer<ExternalWorkerService> server = new
ExternalWorkerService(options).start()) {
+ System.out.format(
+ "External worker service started at address: %s",
+ server.getApiServiceDescriptor().getUrl());
+ while (true) {
+ // Wait indefinitely to keep ExternalWorkerService running
+ Sleeper.DEFAULT.sleep(60 * 60 * 24 * 1000);
Review comment:
If there's no reason to loop, let's make it a really long sleep to
simplify the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
