Singleton MongoClient instance provider

Question

A Java service that connects to MongoDB in production, but opened connection count is too much. It affects on MongoDb performance, so I have come up with a solution that provides only one MongoClient instance so which will reduce the connections.

Here is my code, is it a good way to provide a singleton object from 3rd party library?

import com.mongodb.client.MongoClient;
import com.mongodb.client.MongoClients;

public class MongoClientFactory {

    private static volatile MongoClient instance = null;

    private MongoClientFactory() {}

    public static MongoClient getInstance(String connectionString) {
        if (instance == null) {
            synchronized(MongoClientFactory.class) {
                if (instance == null) {
                    instance = MongoClients.create(connectionString);
                }
            }
        }
        return instance;
    }
}

Answer 1

There are already links on this page to reasons why singletons should not be used. I will not repeat those.

is it a good way to provide a singleton object from 3rd party library?

No. Your singleton has an undocumented internal state which affects the processing of the input parameters. On the first invocation, the getInstance(String connectionString) methods returns a connection to the database requested by the caller. On subsequent invocations it returns a connection to the database requested by the first caller, regardless of what connection string is provided to it. You could document it but it doesn't fix the issue of this being a very error prone solution: your method does something unexpected if something that cannot be verified during run time happened before it was called.

Answer 2

I am not expert in singleton but does the connectionString really change over the life of the application? Usually (I might be wrong) this string is declared with the final keyword.

Answer 3

With your code you'll get getInstance(database1) == getInstance(database2) which is obviously wrong, this points to a problem with your API. Using a singleton (poorly) is the likely cause here. There's good reason why singletons are discouraged.

For your problem you really should have one instance created in your main that's then passed through to all objects that need it and the instance is then kept alive. In general what you're looking for is called Dependency Injection and there are several frameworks to make it easier, for example dagger and guice.