From b7c0bc468f2f7ded69edeb0c4ee2366941dff28a Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 4 Sep 2023 17:42:02 +0100 Subject: [PATCH] WIP: Create indexes on foreign key columns if needed. --- .../org/briarproject/bramble/db/Database.java | 3 + .../bramble/db/DatabaseTypes.java | 6 +- .../briarproject/bramble/db/H2Database.java | 6 +- .../bramble/db/HyperSqlDatabase.java | 6 +- .../briarproject/bramble/db/JdbcDatabase.java | 142 +++++++++++++++++- .../bramble/db/SqliteDatabase.java | 6 +- .../bramble/db/HyperSqlDatabaseTest.java | 7 +- .../bramble/db/JdbcDatabaseTest.java | 15 ++ 8 files changed, 178 insertions(+), 13 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java index 418770eac..056261783 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java @@ -413,6 +413,9 @@ interface Database { */ Collection getMessageIds(T txn, GroupId g) throws DbException; + Collection explainGetMessageIds(T txn, GroupId g) + throws DbException; + /** * Returns the IDs of any delivered messages in the given group with * metadata that matches all entries in the given query. If the query is diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseTypes.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseTypes.java index 55a557d75..f9bb455a8 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseTypes.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseTypes.java @@ -4,14 +4,16 @@ class DatabaseTypes { private final String hashType, secretType, binaryType; private final String counterType, stringType; + private final String explainCommand; // FIXME: Remove public DatabaseTypes(String hashType, String secretType, String binaryType, - String counterType, String stringType) { + String counterType, String stringType, String explainCommand) { this.hashType = hashType; this.secretType = secretType; this.binaryType = binaryType; this.counterType = counterType; this.stringType = stringType; + this.explainCommand = explainCommand; } /** @@ -22,6 +24,7 @@ class DatabaseTypes { *
  • _BINARY *
  • _COUNTER *
  • _STRING + *
  • _EXPLAIN */ String replaceTypes(String s) { s = s.replaceAll("_HASH", hashType); @@ -29,6 +32,7 @@ class DatabaseTypes { s = s.replaceAll("_BINARY", binaryType); s = s.replaceAll("_COUNTER", counterType); s = s.replaceAll("_STRING", stringType); + s = s.replaceAll("_EXPLAIN", explainCommand); return s; } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/H2Database.java b/bramble-core/src/main/java/org/briarproject/bramble/db/H2Database.java index ff13897d3..36e7d98fa 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/H2Database.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/H2Database.java @@ -42,8 +42,10 @@ class H2Database extends JdbcDatabase { private static final String COUNTER_TYPE = "INT NOT NULL AUTO_INCREMENT PRIMARY KEY"; private static final String STRING_TYPE = "VARCHAR"; + private static final String EXPLAIN_COMMAND = "EXPLAIN"; private static final DatabaseTypes dbTypes = new DatabaseTypes(HASH_TYPE, - SECRET_TYPE, BINARY_TYPE, COUNTER_TYPE, STRING_TYPE); + SECRET_TYPE, BINARY_TYPE, COUNTER_TYPE, STRING_TYPE, + EXPLAIN_COMMAND); private final DatabaseConfig config; private final String url; @@ -74,7 +76,7 @@ class H2Database extends JdbcDatabase { boolean reopen = isNonEmptyDirectory(dir); if (LOG.isLoggable(INFO)) LOG.info("Reopening DB: " + reopen); if (!reopen && dir.mkdirs()) LOG.info("Created database directory"); - super.open("org.h2.Driver", reopen, key, listener); + super.open("org.h2.Driver", reopen, false, key, listener); if (LOG.isLoggable(INFO)) { LOG.info("Contents of account directory after opening DB:"); logFileOrDir(LOG, INFO, dir.getParentFile()); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/HyperSqlDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/HyperSqlDatabase.java index 87051cc12..e10a37ae5 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/HyperSqlDatabase.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/HyperSqlDatabase.java @@ -41,8 +41,10 @@ class HyperSqlDatabase extends JdbcDatabase { private static final String COUNTER_TYPE = "INTEGER NOT NULL" + " PRIMARY KEY GENERATED ALWAYS AS IDENTITY(START WITH 1)"; private static final String STRING_TYPE = "VARCHAR"; + private static final String EXPLAIN_COMMAND = "EXPLAIN PLAN FOR"; private static final DatabaseTypes dbTypes = new DatabaseTypes(HASH_TYPE, - SECRET_TYPE, BINARY_TYPE, COUNTER_TYPE, STRING_TYPE); + SECRET_TYPE, BINARY_TYPE, COUNTER_TYPE, STRING_TYPE, + EXPLAIN_COMMAND); private final DatabaseConfig config; private final String url; @@ -70,7 +72,7 @@ class HyperSqlDatabase extends JdbcDatabase { boolean reopen = isNonEmptyDirectory(dir); if (LOG.isLoggable(INFO)) LOG.info("Reopening DB: " + reopen); if (!reopen && dir.mkdirs()) LOG.info("Created database directory"); - super.open("org.hsqldb.jdbc.JDBCDriver", reopen, key, listener); + super.open("org.hsqldb.jdbc.JDBCDriver", reopen, true, key, listener); return reopen; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java index 9ad551ca8..05bb2af46 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java @@ -359,10 +359,84 @@ abstract class JdbcDatabase implements Database { + " ON messages (cleanupDeadline)"; // FIXME: Migration needs to add new index - private static final String INDEX_OUTGOING_KEYS_BY_TRANSPORT_ID_KEYSET_ID = - "CREATE INDEX IF NOT EXISTS outgoingKeysByTransportIdKeysetId" + private static final String INDEX_OUTGOING_KEYS_BY_TRANSPORT_ID_KEY_SET_ID = + "CREATE INDEX IF NOT EXISTS outgoingKeysByTransportIdKeySetId" + " ON outgoingKeys (transportId, keySetId)"; + private static final String FOREIGN_INDEX_CONTACTS_BY_LOCAL_AUTHOR_ID = + "CREATE INDEX IF NOT EXISTS contactsByLocalAuthorId" + + " ON contacts (localAuthorId)"; + + private static final String FOREIGN_INDEX_GROUP_METADATA_BY_GROUP_ID = + "CREATE INDEX IF NOT EXISTS groupMetadataByGroupId" + + " ON groupMetadata (groupId)"; + + private static final String FOREIGN_INDEX_GROUP_VISIBILITIES_BY_CONTACT_ID = + "CREATE INDEX IF NOT EXISTS groupVisibilitiesByContactId" + + " ON groupVisibilities (contactId)"; + + private static final String FOREIGN_INDEX_GROUP_VISIBILITIES_BY_GROUP_ID = + "CREATE INDEX IF NOT EXISTS groupVisibilitiesByGroupId" + + " ON groupVisibilities (groupId)"; + + private static final String FOREIGN_INDEX_MESSAGES_BY_GROUP_ID = + "CREATE INDEX IF NOT EXISTS messagesByGroupId" + + " ON messages (groupId)"; + + private static final String FOREIGN_INDEX_MESSAGE_METADATA_BY_MESSAGE_ID = + "CREATE INDEX IF NOT EXISTS messageMetadataByMessageId" + + " ON messageMetadata (messageId)"; + + private static final String FOREIGN_INDEX_MESSAGE_METADATA_BY_GROUP_ID = + "CREATE INDEX IF NOT EXISTS messageMetadataByGroupId" + + " ON messageMetadata (groupId)"; + + private static final String FOREIGN_INDEX_MESSAGE_DEPENDENCIES_BY_GROUP_ID = + "CREATE INDEX IF NOT EXISTS messageDependenciesByGroupId" + + " ON messageDependencies (groupId)"; + + private static final String + FOREIGN_INDEX_MESSAGE_DEPENDENCIES_BY_MESSAGE_ID = + "CREATE INDEX IF NOT EXISTS messageDependenciesByMessageId" + + " ON messageDependencies (messageId)"; + + private static final String FOREIGN_INDEX_OFFERS_BY_CONTACT_ID = + "CREATE INDEX IF NOT EXISTS offersByContactId" + + " ON offers (contactId)"; + + private static final String FOREIGN_INDEX_STATUSES_BY_MESSAGE_ID = + "CREATE INDEX IF NOT EXISTS statusesByMessageId" + + " ON statuses (messageId)"; + + private static final String FOREIGN_INDEX_STATUSES_BY_CONTACT_ID = + "CREATE INDEX IF NOT EXISTS statusesByContactId" + + " ON statuses (contactId)"; + + private static final String FOREIGN_INDEX_STATUSES_BY_GROUP_ID = + "CREATE INDEX IF NOT EXISTS statusesByGroupId" + + " ON statuses (groupId)"; + + private static final String FOREIGN_INDEX_OUTGOING_KEYS_BY_TRANSPORT_ID = + "CREATE INDEX IF NOT EXISTS outgoingKeysByTransportId" + + " ON outgoingKeys (transportId)"; + + private static final String FOREIGN_INDEX_OUTGOING_KEYS_BY_CONTACT_ID = + "CREATE INDEX IF NOT EXISTS outgoingKeysByContactId" + + " ON outgoingKeys (contactId)"; + + private static final String + FOREIGN_INDEX_OUTGOING_KEYS_BY_PENDING_CONTACT_ID = + "CREATE INDEX IF NOT EXISTS outgoingKeysByPendingContactId" + + " ON outgoingKeys (pendingContactId)"; + + private static final String FOREIGN_INDEX_INCOMING_KEYS_BY_TRANSPORT_ID = + "CREATE INDEX IF NOT EXISTS incomingKeysByTransportId" + + " ON incomingKeys (transportId)"; + + private static final String FOREIGN_INDEX_INCOMING_KEYS_BY_KEY_SET_ID = + "CREATE INDEX IF NOT EXISTS incomingKeysByKeySetId" + + " ON incomingKeys (keySetId)"; + private static final Logger LOG = getLogger(JdbcDatabase.class.getName()); @@ -398,6 +472,7 @@ abstract class JdbcDatabase implements Database { } protected void open(String driverClass, boolean reopen, + boolean createForeignKeyIndexes, @SuppressWarnings("unused") SecretKey key, @Nullable MigrationListener listener) throws DbException { // Load the JDBC driver @@ -424,7 +499,7 @@ abstract class JdbcDatabase implements Database { if (LOG.isLoggable(INFO)) { LOG.info("db dirty? " + wasDirtyOnInitialisation); } - createIndexes(txn); + createIndexes(txn, createForeignKeyIndexes); setDirty(txn, true); commitTransaction(txn); } catch (DbException e) { @@ -557,7 +632,8 @@ abstract class JdbcDatabase implements Database { } } - private void createIndexes(Connection txn) throws DbException { + private void createIndexes(Connection txn, boolean createForeignKeyIndexes) + throws DbException { Statement s = null; try { s = txn.createStatement(); @@ -569,7 +645,31 @@ abstract class JdbcDatabase implements Database { s.executeUpdate(INDEX_STATUSES_BY_CONTACT_ID_TIMESTAMP); s.executeUpdate(INDEX_STATUSES_BY_CONTACT_ID_TX_COUNT_TIMESTAMP); s.executeUpdate(INDEX_MESSAGES_BY_CLEANUP_DEADLINE); - s.executeUpdate(INDEX_OUTGOING_KEYS_BY_TRANSPORT_ID_KEYSET_ID); + s.executeUpdate(INDEX_OUTGOING_KEYS_BY_TRANSPORT_ID_KEY_SET_ID); + // Some DB implementations automatically create indexes on columns + // that are foreign keys, others don't + if (createForeignKeyIndexes) { + s.executeUpdate(FOREIGN_INDEX_CONTACTS_BY_LOCAL_AUTHOR_ID); + s.executeUpdate(FOREIGN_INDEX_GROUP_METADATA_BY_GROUP_ID); + s.executeUpdate(FOREIGN_INDEX_GROUP_VISIBILITIES_BY_CONTACT_ID); + s.executeUpdate(FOREIGN_INDEX_GROUP_VISIBILITIES_BY_GROUP_ID); + s.executeUpdate(FOREIGN_INDEX_MESSAGES_BY_GROUP_ID); + s.executeUpdate(FOREIGN_INDEX_MESSAGE_METADATA_BY_MESSAGE_ID); + s.executeUpdate(FOREIGN_INDEX_MESSAGE_METADATA_BY_GROUP_ID); + s.executeUpdate(FOREIGN_INDEX_MESSAGE_DEPENDENCIES_BY_GROUP_ID); + s.executeUpdate( + FOREIGN_INDEX_MESSAGE_DEPENDENCIES_BY_MESSAGE_ID); + s.executeUpdate(FOREIGN_INDEX_OFFERS_BY_CONTACT_ID); + s.executeUpdate(FOREIGN_INDEX_STATUSES_BY_MESSAGE_ID); + s.executeUpdate(FOREIGN_INDEX_STATUSES_BY_CONTACT_ID); + s.executeUpdate(FOREIGN_INDEX_STATUSES_BY_GROUP_ID); + s.executeUpdate(FOREIGN_INDEX_OUTGOING_KEYS_BY_TRANSPORT_ID); + s.executeUpdate(FOREIGN_INDEX_OUTGOING_KEYS_BY_CONTACT_ID); + s.executeUpdate( + FOREIGN_INDEX_OUTGOING_KEYS_BY_PENDING_CONTACT_ID); + s.executeUpdate(FOREIGN_INDEX_INCOMING_KEYS_BY_TRANSPORT_ID); + s.executeUpdate(FOREIGN_INDEX_INCOMING_KEYS_BY_KEY_SET_ID); + } s.close(); } catch (SQLException e) { tryToClose(s, LOG, WARNING); @@ -1920,6 +2020,38 @@ abstract class JdbcDatabase implements Database { } } + @Override + public Collection explainGetMessageIds(Connection txn, GroupId g) + throws DbException { + PreparedStatement ps = null; + ResultSet rs = null; + try { + String sql = dbTypes.replaceTypes("_EXPLAIN SELECT messageId" + + " FROM messages" + + " WHERE groupId = ? AND state = ?"); + ps = txn.prepareStatement(sql); + ps.setBytes(1, g.getBytes()); + ps.setInt(2, DELIVERED.getValue()); + rs = ps.executeQuery(); + int cols = rs.getMetaData().getColumnCount(); + List explanation = new ArrayList<>(); + while (rs.next()) { + StringBuilder sb = new StringBuilder(); + for (int i = 1; i <= cols; i++) { + sb.append(rs.getString(i)).append(' '); + } + explanation.add(sb.toString()); + } + rs.close(); + ps.close(); + return explanation; + } catch (SQLException e) { + tryToClose(rs, LOG, WARNING); + tryToClose(ps, LOG, WARNING); + throw new DbException(e); + } + } + @Override public Collection getMessageIds(Connection txn, GroupId g, Metadata query) throws DbException { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/SqliteDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/SqliteDatabase.java index 015f34d3e..940ca05eb 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/SqliteDatabase.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/SqliteDatabase.java @@ -41,8 +41,10 @@ class SqliteDatabase extends JdbcDatabase { private static final String COUNTER_TYPE = "INTEGER PRIMARY KEY AUTOINCREMENT"; private static final String STRING_TYPE = "VARCHAR"; + private static final String EXPLAIN_COMMAND = "EXPLAIN QUERY PLAN"; private static final DatabaseTypes dbTypes = new DatabaseTypes(HASH_TYPE, - SECRET_TYPE, BINARY_TYPE, COUNTER_TYPE, STRING_TYPE); + SECRET_TYPE, BINARY_TYPE, COUNTER_TYPE, STRING_TYPE, + EXPLAIN_COMMAND); private final DatabaseConfig config; private final String url; @@ -71,7 +73,7 @@ class SqliteDatabase extends JdbcDatabase { boolean reopen = isNonEmptyDirectory(dir); if (LOG.isLoggable(INFO)) LOG.info("Reopening DB: " + reopen); if (!reopen && dir.mkdirs()) LOG.info("Created database directory"); - super.open("org.sqlite.JDBC", reopen, key, listener); + super.open("org.sqlite.JDBC", reopen, true, key, listener); return reopen; } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlDatabaseTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlDatabaseTest.java index 90fe79bc9..10b598367 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlDatabaseTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/HyperSqlDatabaseTest.java @@ -18,6 +18,11 @@ public class HyperSqlDatabaseTest extends JdbcDatabaseTest { @Override protected JdbcDatabase createDatabase(DatabaseConfig config, MessageFactory messageFactory, Clock clock) { - return new HyperSqlDatabase(config, messageFactory ,clock); + return new HyperSqlDatabase(config, messageFactory, clock); + } + + @Override + public void testExplainGetMessageIds() { + // Ugh, HSQLDB can't handle EXPLAIN PLAN FOR in prepared statements } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java index be14b8b5a..82d80d7fc 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java @@ -2500,6 +2500,21 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(NO_CLEANUP_DEADLINE, db.getNextCleanupDeadline(txn)); } + // FIXME: Remove + @Test + public void testExplainGetMessageIds() throws Exception { + Database db = open(false); + Connection txn = db.startTransaction(); + db.addGroup(txn, group); + Collection explanation = db.explainGetMessageIds(txn, groupId); + db.commitTransaction(txn); + db.close(); + + System.out.println("getMessageIds(T, GroupId)"); + for (String line : explanation) System.out.println(line); + System.out.println(); + } + private Database open(boolean resume) throws Exception { return open(resume, new TestMessageFactory(), new SystemClock()); }