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

InfluxDB Client prevents application from exiting when batch mode is enabled #762

Open
adyang opened this issue Aug 14, 2021 · 3 comments
Open

Comments

@adyang
Copy link
Contributor

adyang commented Aug 14, 2021

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).

@majst01
Copy link
Collaborator

majst01 commented Aug 15, 2021

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.

@adyang
Copy link
Contributor Author

adyang commented Aug 18, 2021

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?

@majst01
Copy link
Collaborator

majst01 commented Aug 19, 2021

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

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

No branches or pull requests

2 participants