blacklist with wildcard using Lucene

Question

I have a table that contains information about users, which I deny loading a specific web page.

Each record contains 4 columns: referrer, ip, userAgent & email.

Each column can contain a wild card (i.e. an asterisk *), which make me block a group.

How could performance be improved with this code?

import org.apache.lucene.analysis.standard.StandardAnalyzer;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.StringField;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexWriterConfig;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.store.RAMDirectory;

import java.io.IOException;

class BlacklistLucene {

    private static final String MATCH_ALL = "*";
    private static IndexSearcher cache;

    private enum Fields {
        referrer, ip, userAgent, email
    }

    private static Document getDocument(String referrer, String ip, String userAgent, String email) {
        Document doc = new Document();
        doc.add(getStringField(referrer, Fields.referrer.name()));
        doc.add(getStringField(ip, Fields.ip.name()));
        doc.add(getStringField(userAgent, Fields.userAgent.name()));
        doc.add(getStringField(email, Fields.email.name()));
        return doc;
    }

    private static StringField getStringField(String val, String field) {
        return new StringField(field, val, Field.Store.NO);
    }

    private static BooleanQuery createQuery(String referrer, String ip, String userAgent, String email) {
        return new BooleanQuery.Builder()
                .add(createBooleanQuery(Fields.referrer.name(), referrer), BooleanClause.Occur.FILTER)
                .add(createBooleanQuery(Fields.ip.name(), ip), BooleanClause.Occur.FILTER)
                .add(createBooleanQuery(Fields.userAgent.name(), userAgent), BooleanClause.Occur.FILTER)
                .add(createBooleanQuery(Fields.email.name(), email), BooleanClause.Occur.FILTER)
                .build();
    }

    private static BooleanQuery createBooleanQuery(String key, String value) {
        return new BooleanQuery.Builder()
                .add(new TermQuery(new Term(key, value)), BooleanClause.Occur.SHOULD)
                .add(new TermQuery(new Term(key, MATCH_ALL)), BooleanClause.Occur.SHOULD)
                .build();
    }

    private static boolean isBlacklisted(String ref, String ip, String ue, String email) throws IOException {
        BooleanQuery query = createQuery(ref, ip, ue, email);
        return cache.search(query, 1).totalHits > 0;
    }


    public static void main(String[] args) throws Exception {

        RAMDirectory directory = new RAMDirectory();
        IndexWriter writer = new IndexWriter(directory, new IndexWriterConfig(new StandardAnalyzer()));
        writer.addDocument(getDocument("ref1", "127.0.0.ip1", "Mozilla UserAgent1", "email.com"));
        writer.addDocument(getDocument("ref2", "127.0.0.ip2", "Mozilla UserAgent2", "*"));
        writer.close();
        DirectoryReader reader = DirectoryReader.open(directory);
        cache = new IndexSearcher(reader);

        System.out.println(isBlacklisted("ref1", "127.0.0.ip1", "Mozilla UserAgent1", "email.com")); // true
        System.out.println(isBlacklisted("r2.com", "127.0.0.4", "Mozilla", "yahoo.com")); // false
        System.out.println(isBlacklisted("ref2", "127.0.0.ip2", "Mozilla UserAgent2", "this is ignored")); // true
        System.out.println(isBlacklisted("*", "127.0.0.ip2", "Mozilla UserAgent2", "*")); // true
    }
}

Answer 1

private enum Fields {
    referrer, ip, userAgent, email

It is customary to use UPPERCASE for enum members.

private static Document getDocument(String referrer, String ip, String userAgent, String email) {

I wouldn't mind seeing a javadoc somewhere that explains what a referrer URL looks like. It can be quite detailed, including ? query terms, but I imagine you strip it down to page or FQDN level, or perhaps even to 2nd-level domain, which .co.uk and others add interesting wrinkles to.

Similarly, I expect ip looks like "10.0.1.2", but some folks prefer the sort order they get from normalizing to "010.000.001.002" or compact hex "0A000102". And the UA might be case-smashed or truncated, and (at least the 2nd half of) the email might be case-smashed, as well.

Some folks choose to blacklist a prefix rather than a /32 individual IP, e.g. matching first three bytes for a /24, or masking for a /20. If you could typically combine 512 IPs into a /23 prefix, your DB would shrink by orders of magnitude, making cache hits go up and random I/Os go down, supporting higher query rates.

I see no evidence that your server answers IPv6 clients. Handling prefixes would be even more important in that environment.

Kudos for following Webster, despite history. Oh, actually, wait! Does Fields.referrer.name() even work? Or does it return one more "r" than headers arriving off-the-wire have? Do tests succeed?

Thank you for adding the getStringField() helper - it improves readability. I wonder if we could achieve even greater conciseness by putting it in the Fields class?

            .add(new TermQuery(new Term(key, value)), BooleanClause.Occur.SHOULD)

I'm reading BooleanClause.Occur :

SHOULD. Use this operator for clauses that should appear in the matching documents. For a BooleanQuery with no MUST clauses one or more SHOULD clauses must match a document for the BooleanQuery to match.

I wouldn't mind seeing a comment explaining why SHOULD works better here. The simplest thing to do seems like MUST - then the Gentle Reader wouldn't be puzzling out what other interactions there might be, what might override it.

private static boolean isBlacklisted(String ref, String ip, String ue, String email) throws IOException {

The ue identifier is very odd. A more natural choice would be ua.

    RAMDirectory directory = new RAMDirectory();

Rather than invent a name for this temp, you might have just buried it in the IndexWriter() call. Then I don't have to worry about other interactions further down in the code. Code with fewer variables in-scope is code that is more easily read at a glance.

    cache = new IndexSearcher(reader);

I would have preferred to see this as a static initializer, rather than deferring until main runs, to make your public API easier to use and so more reusable. Static initializers run at class load time, which can threaten to introduce cyclic dependencies among your own classes, but here the app and the library are well separated so I don't have that concern.

    System.out.println(isBlacklisted("ref1", "127.0.0.ip1", "Mozilla UserAgent1", "email.com")); // true
    System.out.println(isBlacklisted("r2.com", "127.0.0.4", "Mozilla", "yahoo.com")); // false

Well! Now we finally get down to brass tacks, answering some doubts I voiced above. Ok, first thing, it's nice to see the test results printed, but it would so much nicer to have a self-evaluating test, e.g. assertTrue in JUnit.

POLA violation: I was not expecting that "ref1" might be a plausible valid input. Yes, I understand you're making up test inputs. But that's not even an FQDN. Maybe I should understand the substring "hoo" would match "yahoo.com"? When I saw referrer I was kind of mislead into thinking in RFC terms, rather than grep -F terms. At some point you'll blacklist "oo.com" and then see interesting interactions.

Similarly "ip1" was unexpected, and while "Mozilla" makes perfect sense the longer UAs are very test oriented, given that matching a structured user-agent is complex in its own right. Consider pre-processing "equivalent" UAs down to a single-word identifier before handing them to Lucene.

Test coverage seems light. At a bare minimum you probably want four or eight tests, each with three wildcards, so you can focus on individual field behavior.

Test cases are an opportunity to educate folks about your public API, just as javadoc offers an opportunity to educate. Good test data can help to reveal your intent.

It's hard to comment on performance when it isn't clear what semantics you support (e.g. substring vs. prefix-match) and what Lucene features you considered using to support performant queries. Add some benchmark results, and contrast e.g. "Lucene answers 20 query/second" with a requirement of "peak web traffic is 10 hit/sec" or "is 30 hit/sec" so we can see any gap. Mention Winchester vs. SSD, and available RAM. It would be useful for IddoE to create a separate Answer posting that describes any recent work with this code.

Caching is a big deal for performance, so you will need a representative workload, either synthetic or gathered from production. Issuing 20 high-entropy queries each second is very different from 20 predictable queries.

Lucene is very nice, and can be fast. In createQuery() it's not obvious to me that trusting Lucene to optimize MATCH_ALL will yield fastest results. You could just suppress such terms, so Lucene considers between 1 and 4 fields according to how many MATCH_ALL's you used.