spurious DatabaseCorruptError with boolean term query and sparse docids

[Dan Callaghan]

The Sup email client (​https://supmua.dev/) uses Xapian to index emails. Every email is indexed as a document with a Kmail term. Sup also uses an unusual docid assignment scheme: docids are assigned based on the email's date in reverse order, starting from the year 2038 with the docid increasing as the date goes further back in time to 1970. Sup then performs boolean queries in ascending docid order, to find most recent emails first.

This docid assignment scheme means that the docid numbering space is non-contiguous and spread over a wide range. Sup users may have millions of messages covering decades.

When the user asks Sup to list all mail, Sup performs a query for the Kmail term. When retrieving more than a certain number of results (in my testing around 400) Xapian raises a spurious DatabaseCorruptError, even though the database is not actually corrupt according to xapian-check.

This has been reported in Sup as: ​https://github.com/sup-heliotrope/sup/issues/628

The spurious exception is reproducible with some straightforward Xapian operations, using the unusual reverse docid assignment behaviour mimicking Sup:

#include <xapian.h>
#include <cstdio>
#include <cstdlib>

int main() {
    system("rm -rf /tmp/xapian-bug-repro-db");
    Xapian::WritableDatabase db("/tmp/xapian-bug-repro-db",
                                Xapian::DB_CREATE_OR_OPEN);

    const Xapian::docid TOPID = 2000000;
    const unsigned N_DOCS = 2000;
    for (unsigned i = 0; i < N_DOCS; i++) {
        Xapian::Document doc;
        doc.add_term("Kmail", 0);
        db.replace_document(TOPID - i, doc);
    }
    db.commit();

    try {
        Xapian::Enquire enquire(db);
        enquire.set_weighting_scheme(Xapian::BoolWeight());
        enquire.set_query(Xapian::Query("Kmail"));
        Xapian::MSet mset = enquire.get_mset(0, N_DOCS);
        printf("OK size=%u\n", unsigned(mset.size()));
    } catch (const Xapian::Error& e) {
        printf("CRASH: %s: %s\n", e.get_type(), e.get_msg().c_str());
        return 1;
    }
    return 0;
}

This fails with Xapian 1.4.31:

$ ./xapian-repro
CRASH: DatabaseCorruptError: Unexpected end of posting list for 'Kmail'

[Olly Betts]

Thanks, I can reproduce with git main.

It doesn't happen with an inmemory DB created like so:

    Xapian::WritableDatabase db("", Xapian::DB_BACKEND_INMEMORY);

I'm fairly sure it's going to be glass-specific, so marking as such.

It also fails if I query the same database opened as a Database, so it's not some sort of inconsistent state in the WritableDatabase object. Compacting it gives a database which works, which is interesting.

I'll investigate more deeply.

Every email is indexed as a document with a Kmail term

If you have other types of document this suggestion won't work, but if Kmail indexes all documents then a good workaround is to use the query Xapian::Query::MatchAll instead of Xapian::Query("Kmail") - this will actually be a little more efficient too. If this is the case, I'd guess this approach predates Xapian 1.2, as before then MatchAll would have been significantly slower.

[Olly Betts]

Replying to Olly Betts:

Every email is indexed as a document with a Kmail term

If you have other types of document this suggestion won't work, but if Kmail indexes all documents then a good workaround is to use the query Xapian::Query::MatchAll instead of Xapian::Query("Kmail") - this will actually be a little more efficient too. If this is the case, I'd guess this approach predates Xapian 1.2, as before then MatchAll would have been significantly slower.

It looks like Xapian itself actually spots the above would be more efficient and the query optimiser effectively replaces your query with Xapian::Query::MatchAll, which works from the document length data, which is stored in a pseudo-postlist. However it seems when it advances to another chunk it checks for the original term name (i.e. Kmail) rather than the dummy term name which the document length chunks actually have, and the reported corruption is because it thinks it has gone past the last chunk without seeing an "i am the last chunk" flag.

This replacement is only allowed when the query doesn't need wdf values, such as your BoolWeight query.

I think we can just compare the term name for the new chunk against the one from the previous chunk instead.

[Olly Betts]

I've attached a candidate fix for glass which fixes this for me. It's against 2.x and will need adapting a bit for 1.4.x. I need to sort out a regression test and check honey still.

Compacting it gives a database which works, which is interesting.

Compacting to glass with --no-renumber gives a database which still fails (without the patch) though. I think because renumbering shifts all the docids down so the first is 1, and the query optimiser has a special case for a term indexing all docs when they're contiguous and start at 1 which just counts from 1 to N rather than fetching postings from the database, so we don't even get to the buggy code.

Compacting to honey with --no-renumber gives a database which works, but the encodings used are different so it might just be this example doesn't trigger it - I'll have to review the code to see if it has an equivalent issue.

[Dan Callaghan]

Thanks for the quick response!

I confirmed that your suggestion to use MatchAll avoids the issue in Sup: ​https://supmua.dev/git/sup/commit/735593960308648000dbe99d706221c24a306746.html

[Olly Betts]

Essentially the same underlying issue does affect the honey backend, but there it can't cause an exception. It can cause us to switch to the real postlist for the term, but that's just a performance problem rather than a correctness one.

Fixed for both glass and honey in git main by 399bff55a2f5c0f78d43a91b2fba67794be03632.

Fixed on RELEASE/1.4 (for glass only; honey is 2.x only) by 0d2ac53d518fd6ce5f9dfff4f8d3f82bbb758598.

It looks like chert (1.4.x only) has the same DatabaseCorruptError bug as glass, but your reproducer doesn't trigger it. I'll see if I can adjust it so the bug manifests before I apply the equivalent fix.

[Olly Betts]

Replying to Olly Betts:

It looks like chert (1.4.x only) has the same DatabaseCorruptError bug as glass, but your reproducer doesn't trigger it. I'll see if I can adjust it so the bug manifests before I apply the equivalent fix.

It failed in CI - turns out I'd reconfigured my local work directory for RELEASE/1.4 xapian-core to test something and it wasn't in maintainer-mode so the new testcase wasn't actually getting enabled.

Applied the equivalent fix to chert in db04e6e89d13f284b2294f1ad6dbd4ac4860ca2d.