Use Spring Boot ApplicationReadyEvent to start workers

[osialr]

Is your feature request related to a problem? Please describe.
Hi there 👋!

After #1614, users must put applicationContext.start() in unit tests and main methods. Invoking start() is not idiomatic Spring Boot and leads to confusion when workers haven't started. (#1648). Even Spring Boot's Sample Application does not use start()

Describe the solution you'd like
Use Spring Boot's ApplicationReadyEvent instead of Spring's ContextStartedEvent.

As described in Application Events and Listeners, An ApplicationReadyEvent is sent after any application and command-line runners have been called.

This represents the same application state as when SpringApplication.run() has returned and where users are instructed to manually add a call start().

Additionally, using ApplicationReadyEvent removes the need for every test to contain the boilerplate

  @Autowired ConfigurableApplicationContext applicationContext;

  @BeforeEach
  void setUp() {
    applicationContext.start();
  }

Describe alternatives you've considered

• Put big bold text at the top of the README calling out this unusual requirement of a spring-boot library.
• Detect when the user should have put in a call .start() and emit a warning.

Additional context

I have a PR ready to submit if the feature is approved.

[Quinn-With-Two-Ns]

Looking at this issue on the Spring Boot github spring-projects/spring-boot#35936 I think your right that the current approach is not correct.

Is ApplicationReadyEvent the idiomatic Spring Boot way? I looked at the Spring Boot included http servers and did not see it being used there. From the description it sounds too late in the lifecyle to be starting the workers.

This represents the same application state as when SpringApplication.run() has returned and where users are instructed to manually add a call start().

Can you point me towards the docs that suggest this?

[osialr]

Is ApplicationReadyEvent the idiomatic Spring Boot way? I looked at the Spring Boot included http servers and did not see it being used there. From the description it sounds too late in the lifecyle to be starting the workers.

I suggested ApplicationReadyEvent to as closely preserve the existing behavior. It may not be the right phase over something like ApplicationStartedEvent.

One example in spring-boot that uses ApplicationReadyEvent is BackgroundPreinitializer. It starts a thread after ApplicationEnvironmentPreparedEvent then at ApplicationReadyEvent waits for the the initialization to finish if it hasn't already.

This represents the same application state as when SpringApplication.run() has returned and where users are instructed to manually add a call start().

Can you point me towards the docs that suggest this?

In the docs for ApplicationReadyEvent
Event published as late as conceivably possible to indicate that the application is ready to service requests.

There is also a comment in spring-boot#33935:
The ApplicationReadyEvent is published to indicate within the application that it is now ready to service requests. This readiness should include all of your application's own components which should have got ready during bean initialization or as application or command-line runners. Once this event has been published and all listeners are aware that servicing of requests may now begin, the AvailabilityChangeEvent is published to make this readiness visible externally so that requests are routed to the application.

I ran an empty spring-boot-starter project with some event handlers and debug logging. The filtered output is

com.example.springboot.Application       : Invoking SpringApplication::run
com.example.springboot.Application       : Starting Application using Java 17.0.8 with PID 730127
com.example.springboot.Application       : Started Application in 0.779 seconds (process running for 1.237)
com.example.springboot.Application       : Received ApplicationStartedEvent
o.s.b.a.ApplicationAvailabilityBean      : Application availability state LivenessState changed to CORRECT
com.example.springboot.Application       : Received AvailabilityChangeEvent
com.example.springboot.Application       : Received ApplicationReadyEvent
o.s.b.a.ApplicationAvailabilityBean      : Application availability state ReadinessState changed to ACCEPTING_TRAFFIC
com.example.springboot.Application       : Received AvailabilityChangeEvent
com.example.springboot.Application       : SpringApplication::run has completed.  Invoking ConfigurableApplicationContext::start
com.example.springboot.Application       : Received ContextStartedEvent
com.example.springboot.Application       : ConfigurableApplicationContext::start has completed

Whether ApplicationReadyEvent or ApplicationStartedEvent is better depends on whether the application LivenessState should be updated before or after the connection has been attempted.

[Quinn-With-Two-Ns]

My concern with ApplicationReadyEvent is this part

The ApplicationReadyEvent is published to indicate within the application that it is now ready to service requests.

Since workers service requests I believe they should be started before this event. Looking at the spring integration with http servers, like tomcat, it looks like they don't listen for either of these events, they just assume people will use sprint bean lifecycles to make sure there beans start before the server if there is a dependency. Perhaps temporal workers autostart should work the same.

[osialr]

There's upsides and downsides to invoking workferFactory.start() within a bean lifecycle instead of on a application event.

The upside is that it's fail-fast and the application will quit if on connection failure. The downside is that there are cases where I want the web servlet to start up even when temporal connection fails. The main one is the /health endpoint) plus custom Health Indicators to monitor system state.

If the application aborts at startup then that I have to dig through the logs instead of using the health endpoint to see what's going wrong. Additionally, external monitoring tools expect to have a health check endpoint. A good connection at startup could always enter a bad state later.

So if you were to go that route, I'd make aborting a configuration option, something like continue-on-failure: true/false. Where the two states are

1. When continue-on-failure is false, call workerFactory.start() in a spring-bean initialization and allow exceptions to propagate.
2. When continue-on-failure is true:, call workerFactory.start() with a try/catch that only logs.

Regardless of the decisions in this ticket, adding a HealthIndicator to this repo would be helpful.

I couldn't deduce why there's a deferred startup, the Starter goes all the way back to the initial project commit. If the goal was to delay start to allow external configurations, the code could be modified to like WebMvcConfigurer.

Something like:

WorkerFactory createWorkerFactory(Collection<WorkerFactoryConfigurer> configurer) {
  WorkerFactory workerFactory = ...; // existing codebase
  
  // Apply custom configuration
  for (var c : configurer) {
     c.doConfig(workerFactory);
  }
  
  // Start factory, no more configuration.
  try {
     workerFactaory.start();
  } catch (Exception e) {
    if (!config.continueOnFailure) {
      throw e;
    }
    LOG.warn("Failed to start temporal", e);
  }

  return workerFactory;
}

Sorry for the long post if you were simply debating ApplicationReadyEvent vs ApplicationStartedEvent 😅

[Quinn-With-Two-Ns]

No I appreciate the long response.

I was debating ApplicationReadyEvent vs ApplicationStartedEvent vs using bean lifecycle. I think another benefit of using bean lifecycles is if a users has beans they want to start after the worker starts that would be easy to setup. That being said I think the inverse (beans start before workers) is more common and all options support that.

Yeah adding some sort of HealthIndicator for temporal spring is reasonable, could you open a separate ticket for that?

[osialr]

another benefit of using bean lifecycles is if a users has beans they want to start after the worker starts that would be easy to setup

True, the event system is kind of a kludge; there's an implicit bound.

But with current spring-boot library deferring .start() until an event, I can add more workers dynamically like in the base example

    WorkerFactory factory = WorkerFactory.newInstance(client);
    Worker yourWorker = factory.newWorker("your_task_queue");

    // Register Workflow
    // and/or register Activities

    factory.start();

with

@Configuration
public class CustomWorkerConfig {
   @AutoWired
   FactoryWorker factoryWorker;

   @PostConstruct
   void addMyWorker() {
     var worker = factoryWorker.newWorker("other_task_queue");
    // Register Workflow
    // and/or register Activities
   }
}

I needed this capability because I am implementing the host specific task queue like in the File Processing example.

If the WorkerFactory is started at the time it becomes a bean, then the above wouldn't work. I'm unsure if there is a lifecycle phase that can call start() and have the bean available to others.

(Also, I opened #1839 about health indicators.)