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.

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

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

Open
infinityat0 opened this issue Nov 8, 2021 · 16 comments · May be fixed by #10701

Comments

@infinityat0
Copy link

infinityat0 commented Nov 8, 2021

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

  • consumer to stop consuming
  • wait until dao is done with the job before shutting down db connection
  • cache to be updated before disconnecting cache
  • produce the last message and only then shutdown producer.

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

@infinityat0
Copy link
Author

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
Copy link
Author

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
Copy link

Is this yet to be triaged?

@jameskleeh
Copy link
Contributor

jameskleeh commented Nov 30, 2021

@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
Copy link
Author

@jameskleeh That may not be ideal because

  • Generally, things related closely are declared together in a constructor. That may not be the order in which they need to be closed
  • This is implicit ordering and not explicit. It's much better to be explicit about what order the user's want.
  • This may work with a single class but not with multiple unrelated classes that have members that need to be ordered (Global ordering)

@mattwelke
Copy link

mattwelke commented Feb 6, 2022

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
Copy link

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
Copy link
Contributor

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
Copy link
Author

infinityat0 commented Apr 7, 2022 via email

@mattwelke
Copy link

mattwelke commented May 20, 2022

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
Copy link
Author

infinityat0 commented Jun 7, 2022 via email

@mattwelke
Copy link

@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
Copy link
Contributor

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

@infinityat0
Copy link
Author

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
Copy link

Any news? Does anyone work on this?

@graemerocher graemerocher linked a pull request Apr 16, 2024 that will close this issue
@sdelamo sdelamo linked a pull request Apr 19, 2024 that will close this issue
@sdelamo
Copy link
Contributor

sdelamo commented May 3, 2024

Any news? Does anyone work on this?

@aleksandy Yes, there is work in progress: #10701

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants