JDBC backend: a catalog table another session created leaves enrolledTrees empty, so that open enrols every tree again

[vharseko]

Lands with #893, which is where openCatalog() and createCatalogTable() are: they are not on master yet.

Problem

openCatalog() makes the catalog of a backend usable once per open of the storage: it reads what the catalog records where the table is there, and creates the table where it is not.

synchronized (catalogLock) {
    if (isExistsTable(catalog)) {
        readEnrolledTrees(catalog);          // fills enrolledTrees from the rows
    } else {
        createCatalogTable(catalog);         // nothing to read from a table just created
    }
    catalogTableOpened = true;
}

The lock orders the transactions of one storage and nothing else, which is why createCatalogTable() tolerates a table that turned up while it was being made — an offline tool beside a running server is a pair no lock of one process can order:

catalogSession.reset();
if (alreadyThere) {                          // isExistsTable(catalog)
    logger.debug("... was created by another session while this one was creating it ...");
    return;                                  // <- returns without reading a row
}

On that path the flag is raised over an empty enrolledTrees, because the branch that reads the rows was not the branch taken. The table is there and full of rows; this storage believes it records nothing.

Consequence

enrolInCatalog() skips a tree the memo already names, and writes it otherwise. With the memo empty, every tree of that open is written again: about 25 upserts and 25 commits on the catalog connection for a stock suffix, each one a row that was already there, and the same for every later write that opens a tree. It self-corrects at the next open of the storage that takes the reading branch.

Not a correctness bug — the upsert is idempotent, the rows are the same rows, and the commits are on the catalog's own connection, so nothing of the caller's transaction is committed and partlyCommitted is not raised. It defeats the optimisation that exists to make an open of a complete catalog write nothing at all.

What the fix looks like

Let the tolerated-race path fall through to readEnrolledTrees(catalog) rather than return: createCatalogTable() answering "the table was there after all" is exactly the condition the reading branch is for. A boolean out of createCatalogTable() — created, or adopted — and openCatalog() reading the rows on the second.

Reachability

Two processes opening the same backend at once: a server starting while an offline tool runs, which is the pair #888 is about, or two servers against one database.