influxdb-java icon indicating copy to clipboard operation
influxdb-java copied to clipboard

InfluxDB Client prevents application from exiting when batch mode is enabled

Open adyang opened this issue 4 years ago • 3 comments

Hi Team,

When an influxdb-java client is created and batch is enabled, it prevents the application from exiting normally due to the use of a default non-daemon thread pool. This prevents the application from shutting down in the event that the main thread exits/ crashes. Adding a shutdown hook would not help with closing the client as the JVM will only run shutdown hooks after all non-daemon thread exits. A small example would be running the following:

public class App {
    public static void main(String[] args) {
        InfluxDB influxDB = InfluxDBFactory.connect("http://localhost:8086");
        influxDB.enableBatch();
        Runtime.getRuntime().addShutdownHook(new Thread(influxDB::close));
        System.out.println(influxDB.ping());
    }
}

Currently, the workaround is to explicitly specify a daemon thread factory:

influxDB.enableBatch(
                BatchOptions.DEFAULTS
                        .threadFactory(runnable -> {
                            Thread thread = new Thread(runnable);
                            thread.setDaemon(true);
                            return thread;
                        })
        );

But I was wondering if it would be a better user experience to have this be the default behaviour so that the main application is allowed to exit normally. The rationale is that a common use case for influxdb-java is to write application metrics and usually one would not expect a metrics library to affect the main application (e.g. cause it to not exit properly).

adyang avatar Aug 14 '21 13:08 adyang

Hi @adyang

nice finding, do you mind adding this to the documentation and probably to the javadoc of the influxDB.enableBatch method ? I'm happy to review a pull request.

majst01 avatar Aug 15 '21 09:08 majst01

Hi alright, just wondering do you think BatchOptions.DEFAULT should have a thread factory that by default produce daemon threads or should this just be documented?

adyang avatar Aug 18 '21 14:08 adyang

As making this default is a change in behavior i really prefer to document it, or if you create a PR to change the behavior for the defaults, this must be documented well in the changelog

majst01 avatar Aug 19 '21 05:08 majst01