micronaut-projects / micronaut-core

Micronaut Application Framework
http://micronaut.io
Apache License 2.0
6.04k stars 1.06k forks source link

Micronaut doesn't give the application code control of the order in which resources are gracefully shutdown/closed #6493

Open infinityat0 opened 2 years ago

infinityat0 commented 2 years ago

Feature description

From conversations with @jameskleeh @jeffbrown it appears that micronaut uses bean dependencies to gracefully shutdown the beans created. This does not follow a functional order of resource termination in a service. Other frameworks do allow for shutdown hooks to be ordered but micronaut doesn't.

Consider the example in the following link. (See: https://twitter.github.io/finatra/user-guide/getting-started/lifecycle.html#server-shutdown-lifecycle)

For example, you have code which is reading from a queue (via a “subscriber”), transforming the data, and then publishing (via a “publisher”) to another queue. When the main application is exiting you most likely want to close the “subscriber” first to ensure that you transform and publish all available data before closing the “publisher”.

Assuming, that both objects are a c.t.util.Closable type, a simple way to close them would be:

closeOnExit(subscriber); closeOnExit(publisher)

With micronaut there doesn't seem to be such control. If I defined a service class like this

   class ServiceClass(kafkaConsumer, kafkaProducer, dao, cache) {
   }

I have no idea in which order (kafkaConsumer, kafkaProducer, dao, and cache) are closed. I may want the following order

and the needs may change in the future but right now, there doesn't seem to be a way to control that.

infinityat0 commented 2 years ago

I have also noticed that doing so myself and listening to a ShutdownEvent wouldn't really work because micronaut doesn't really expose lifecycle methods for a lot of underlying resource handlers. For instance, a kafka producer is created by just calling @KafkaClient annotation and we would not have, in service code any control on when and how we can pause/shut it down.

@Singleton
class ShutdownHooks {

    @EventListener
    fun onShutdownEvent(event: ShutdownEvent) {
        logger.ifInfo { "Received a shutdown event: event=$event" }

        hooks
            .sortedBy { triple -> triple.first }
            .forEach { (order, name, block) ->
                logger.ifInfo { "[$order] Attempting to close $name." }
                try { block() } catch (ex: Exception) {
                    logger.error("Failed to gracefully close $name", ex)
                }
            }
    }

    companion object {
        private val logger = loggerFor<ShutdownHooks>()
        private val hooks = mutableListOf<Triple<Int, String, () -> Unit>>()

        /**
         * Used to register a shutdown hook inside a service.
         *
         * Notes:
         * - Shutdown hooks are called in order mentioned.
         * - These calls are fire and forget.
         * - When multiple shutdown hooks are registered with same order,
         *   we cannot guarantee execution order
         *
         * @param order represents the position in the execution order
         * @param name represents the name of the hook (for logging)
         * @param block the function that needs to be executed upon shutdown
         */
        fun registerHook(order: Int, name: String, block: () -> Unit) = hooks.add(Triple(order, name, block))
    }
}
infinityat0 commented 2 years ago

So, I think the solution here could be a little intrusive for micronaut but quite clean in terms of using it. May be a new annotation like @Closable(int: order) for resources.

  @KafkaClient("foo-client")
  @Closable(order = 3)
  interface FooKafkaProducer {
    ...
  }

  @KafkaListener("bar-client")
  @Closable(order = 1)
  class BarKafkaConsumer {
    ...
  }

  @Closable(order = 2)
  class BazDaoImpl(jdbi: Jdbi): BazDao {
    ...
  }

  @Singleton
  class FooService(consumer: BarKafkaConsumer, dao: BazDao, producer: FooKafkaProducer) {
    ...
  }
avirlrma commented 2 years ago

Is this yet to be triaged?

jameskleeh commented 2 years ago

@infinityat0 What if they were closed in the order they were declared in the constructor? Note that this would be difficult or impossible to guarantee if other beans depended on yours and also the beans in your constructor and they were defined in a different order in the dependent bean

infinityat0 commented 2 years ago

@jameskleeh That may not be ideal because

mattwelke commented 2 years ago

I think this is going to affect us with a Go to Java port we're considering. We use OpenCensus for metrics with a GCP exporter and we need to have our application stop accepting HTTP requests, wait 10 seconds, then perform a flush on the GCP exporter. The 10 second pause is to allow enough time to go by so that there is a 0% chance GCP returns a 429 when the exporter sends its final data point.

If I use the beans approach to manage the HTTP controllers and the metrics exporter, it sounds like I'll run into a problem where because the HTTP controller and the GCP exporter don't depend on each other (with OpenCensus, you go through "views" which are global state), I won't be able to ensure they're shut down in the correct order, with the 10 second pause in the middle. I understand Micronaut has its own approach to metrics that I should look into (Micrometer?) but this lack of control over the lifecycle of the different parts of my app concerns me.

lordbuddha commented 2 years ago

Is there any thoughts/plans on how to progress this.... It is a pretty big omission not to be able to have an ordered shutdown.

graemerocher commented 2 years ago

How about some kind of @DependsOn annotation help here that would ensure that a bean is both created and destroyed prior to a dependent bean? Like:

  @DependsOn(BazDaoImpl.class)
  interface FooKafkaProducer {
    ...
  }

  @KafkaListener("bar-client")
  class BarKafkaConsumer {
    ...
  }

  @DependsOn(BarKafkaConsumer.class)
  class BazDaoImpl(jdbi: Jdbi): BazDao {
    ...
  }

  @Singleton
  class FooService(consumer: BarKafkaConsumer, dao: BazDao, producer: FooKafkaProducer) {
    ...
  }
infinityat0 commented 2 years ago

We are conflating dependencies with order. They may not be the same.

On Thu, Apr 7, 2022, 01:09 Graeme Rocher @.***> wrote:

How about some kind of @DependsOn annotation help here that would ensure that a bean is both created and destroyed prior to a dependent bean? Like:

@DependsOn(BazDaoImpl.class) interface FooKafkaProducer { ... }

@KafkaListener("bar-client") class BarKafkaConsumer { ... }

@DependsOn(BarKafkaConsumer.class) class BazDaoImpl(jdbi: Jdbi): BazDao { ... }

@Singleton class FooService(consumer: BarKafkaConsumer, dao: BazDao, producer: FooKafkaProducer) { ... }

— Reply to this email directly, view it on GitHub https://github.com/micronaut-projects/micronaut-core/issues/6493#issuecomment-1091260487, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6P5JU4HYWZU4ZXB277ELVD2J3HANCNFSM5HT33G7Q . You are receiving this because you were mentioned.Message ID: @.***>

mattwelke commented 2 years ago

Maybe it would make sense to have some kind of event that beans could listen to that signifies application shutdown? And then you could create a bean whose only purpose is to receive this event and call "close" etc methods of your beans in the correct order?

Ex.

class NewMessageQueueReader {
  // depends on writer so that it can write processed messages
  MessageProcessorAndWriter writer;

  close() {
    // Close clients used for writing processed messages (to stop reading new messages)
  }
}
class MessageProcessorAndWriter {
  close() {
    // Wait for in progress processed message writes to finish.
    // Close clients used for writing processed messages
  }
}
class QueueCoordinator [extends ShutDownEventHandler] {
  // should not be new objects, should be the objects actually used in the
  // application to do work
  MessageProcessorAndWriter writer; 
  NewMessageQueueReader reader;

  @Override
  onShutdown() {
    reader.close(); // async so that writer can be closed too
    writer.close();
    // wait til reader and writer closed, meaning all in progress messages
    // were processed and no new messages will come in    
  }
}
infinityat0 commented 2 years ago

That could work and is needed but the unfortunate thing is that beans created by micronaut do not expose lifecycle methods. So, there's not really any control on this from the application perspective.

On Fri, May 20, 2022 at 8:56 AM Matt Welke @.***> wrote:

Maybe it would make more sense to have some kind of event that beans could listen to that signifies application shutdown? And then you could create a bean whose only purpose is to receive this event and call "close" etc methods of your beans in the correct order?

Ex.

class NewMessageQueueReader { ProcessedMessageQueueWriter writer; // depends on writer so that it can write processed messages

close() { // Stop reading messages, close connections } }

class ProcessedMessageQueueWriter { close() { // Stop writing messages, close connections } }

class QueueCoordinator [extends ShutDownEventHandler] { ProcessedMessageQueueWriter writer; NewMessageQueueReader reader;

@Override onShutdown() { writer.close(); // wait til writer closed reader.close(); // wait til reader closed } }

— Reply to this email directly, view it on GitHub https://github.com/micronaut-projects/micronaut-core/issues/6493#issuecomment-1133065892, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6P5KTX2NKHB4DZIRQQKTVK6Y4BANCNFSM5HT33G7Q . You are receiving this because you were mentioned.Message ID: @.***>

-- +Sunny

mattwelke commented 2 years ago

@infinityat0 I think what @graemerocher was suggesting:

How about some kind of @DependsOn annotation help here that would ensure that a bean is both created and destroyed prior to a dependent bean?

Is similar to something that Spring has right now: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/context/annotation/DependsOn.html

A depends-on declaration can specify both an initialization-time dependency and, in the case of singleton beans only, a corresponding destruction-time dependency. Dependent beans that define a depends-on relationship with a given bean are destroyed first, prior to the given bean itself being destroyed. Thus, a depends-on declaration can also control shutdown order.

I noticed this today when exploring Spring Boot docs.

We are conflating dependencies with order. They may not be the same.

The way it's described in the Spring docs, it's about creation and shutdown order, not how dependencies are wired up. I have yet to try this out, but it sounds like this may be what we're looking for. Perhaps a Micronaut @DependsOn annotation could be implemented to be about creation and shutdown order too.

graemerocher commented 2 years ago

For me this seems to be the logical approach but it others disagree would love to hear why

infinityat0 commented 2 years ago

I think @dependsOn is a decent step forward than NOT having any control over lifecycle ordering of beans. I'd argue against the naming because that's misleading. But Alas! 1. I'd much rather have something than nothing and 2. I'd rather it is consistent with other similar frameworks.

But tbh, I feel quite dirty overloading @dependsOn with bean dependency ordering AND lifecycle ordering.

aleksandy commented 5 months ago

Any news? Does anyone work on this?

sdelamo commented 4 months ago

Any news? Does anyone work on this?

@aleksandy Yes, there is work in progress: https://github.com/micronaut-projects/micronaut-core/pull/10701