4

I put my couchbase initialization code inside a static code block:

static {
        initCluster();
        bucket = initBucket("graph");
        metaBucket = initBucket("meta");
        BLACKLIST = new SetObservingCache<String>(() -> getBlackList(), BLACKLIST_REFRESH_INTERVAL_SEC * 1000); 
       }

I know it's not a good practice but it was very convenient and served its purpose, as I need this code to run exactly once in a multi-threaded environment and block all subsequent calls from other threads until it's finished (blacklist has been initialized).

To my surprise, the call to getBlacklist() timed out and couldn't be completed. However, when calling it again after 2 minutes (that's what the ObservingCache does), it completed in less than a second.

In order to solve this, I refactored my code and made the blacklist acquisition lazy:

    public boolean isBlacklisted(String key) {
        // BLACKLIST variable should NEVER be touched outside of this context.
        assureBlacklistIsPopulated();
        return BLACKLIST != null ? BLACKLIST.getItems().contains(key) : false;
    }

    private void assureBlacklistIsPopulated() {
        if (!ENABLE_BLACKLIST) {
            return;
        }
        if (BLACKLIST == null) {
            synchronized (CouchConnectionManager.class) {
                if (BLACKLIST == null) {
                    BLACKLIST = new SetObservingCache<String>(() -> getBlackList(), BLACKLIST_REFRESH_INTERVAL_SEC * 1000);
                }
            }
        }
    }

The call to isBlacklisted() blocks all other threads that attempt to check if an entry is blacklisted until blacklist is initialized. I'm not a big fan of this solution because it's very verbose and error prone - one might try to read from BLACKLIST without calling assureBlacklistIsPopulated() beforehand.

The static (and non final) fields within the class are as follows:

private static CouchbaseCluster cluster;
private static Bucket bucket;
private static Bucket metaBucket;
private static SetObservingCache<String> BLACKLIST;

I can't figure out why the call succeeded when it wasn't a part of the static initialization block. Is there any known performance-related vulnerability of the static initialization block that I'm not aware of?

EDIT: Added initialization code per request

private Bucket initBucket(String bucketName) {
    while(true) {
        Throwable t = null;
        try {
            ReportableThread.updateStatus("Initializing bucket " + bucketName);
            return cluster.openBucket(bucketName);
        } catch(Throwable t1) {
            t1.printStackTrace();
            t = t1;
        }
        try {
            ReportableThread.updateStatus(String.format("Failed to open bucket: %s reason: %s", bucketName,  t));
            Thread.sleep(500);              
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

private void initCluster() {
    CouchbaseEnvironment env = DefaultCouchbaseEnvironment
            .builder()
            .kvTimeout(MINUTE)
            .connectTimeout(MINUTE)
            .retryStrategy(FailFastRetryStrategy.INSTANCE)
            .requestBufferSize(16384 * 2)
            .responseBufferSize(16384 * 2)
            .build();
    while(true) {
        ReportableThread.updateStatus("Initializing couchbase cluster");
        Throwable t = null;
        try {
            cluster = CouchbaseCluster.create(env, getServerNodes());
            if(cluster != null) {
                return;
            }
        } catch(Throwable t1) {
            t1.printStackTrace();
            t = t1;
        }
        try {
            ReportableThread.updateStatus(String.format("Failed to create connection to couch %s", t));
            Thread.sleep(500);              
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

public Set<String> getBlackList() {
    ReportableThread.updateStatus("Getting black list");
    AbstractDocument<?> abstractDoc = get("blacklist", metaBucket, JsonArrayDocument.class);
    JsonArrayDocument doc = null;
    if (abstractDoc != null && abstractDoc instanceof JsonArrayDocument) {
        doc = (JsonArrayDocument)abstractDoc;
    } else {
        return new HashSet<String>();
    }
    ReportableThread.updateStatus(String.format("%s: Got %d items | sorting items", new Date(System.currentTimeMillis()).toString(), doc.content().size()));
    HashSet<String> ret = new HashSet<String>();
    for (Object string : doc.content()) {
        if (string != null) {
            ret.add(string.toString());             
        }
    }
    return ret;
}
KidCrippler
  • 1,633
  • 2
  • 19
  • 34

1 Answers1

1

1st: you are doing the double-check idiom. That's always bad. Put only one if(BLACKLIST==null) and it must be inside the synchronized.

2nd: the lazy init is fine, but do it in a static getInstance() and NEVER expose the BLACKLIST field.

user2023577
  • 1,752
  • 1
  • 12
  • 23
  • 10x for the response. What's so bad about the double check idiom? Second, I don't recall exposing the BLACKLIST field. Can you answer my original question - why is the code infinitely slower within the static block? – KidCrippler Feb 07 '16 at 15:26
  • I don't know. Did you profile it? The static block is ran while the class is loading, which means a lot of lazy classloading will occur during you code execution. If you run later, the bulk of class loading was done during boot and earlier moments. That all I can guess. You could confirm that hypothesis by examining a verbose class log (see java options) while stepping over your call in a debugger. – user2023577 Feb 07 '16 at 21:16