From 43b437af92f7a073241e83184a67ea898041ff0f Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 26 Feb 2021 12:45:26 +0000 Subject: [PATCH] Group messages by group ID when fetching them from database. --- .../bramble/api/db/DatabaseComponent.java | 2 +- .../bramble/cleanup/CleanupManagerImpl.java | 40 ++++++------------- .../org/briarproject/bramble/db/Database.java | 3 +- .../bramble/db/DatabaseComponentImpl.java | 4 +- .../briarproject/bramble/db/JdbcDatabase.java | 16 +++++--- .../bramble/db/JdbcDatabaseTest.java | 4 +- 6 files changed, 30 insertions(+), 39 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java index cf6bfb61a..b5cf1d617 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java @@ -342,7 +342,7 @@ public interface DatabaseComponent extends TransactionManager { *

* Read-only. */ - Map getMessagesToDelete(Transaction txn) + Map> getMessagesToDelete(Transaction txn) throws DbException; /** diff --git a/bramble-core/src/main/java/org/briarproject/bramble/cleanup/CleanupManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/cleanup/CleanupManagerImpl.java index 5cc70dbad..b87c53f4c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/cleanup/CleanupManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/cleanup/CleanupManagerImpl.java @@ -20,9 +20,7 @@ import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.TaskScheduler; import org.briarproject.bramble.api.versioning.ClientMajorVersion; -import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Map.Entry; @@ -132,39 +130,25 @@ class CleanupManagerImpl implements CleanupManager, Service, EventListener { } private void deleteMessages(Transaction txn) throws DbException { - Map clientCache = new HashMap<>(); - Map> deleted = new HashMap<>(); - Map ids = db.getMessagesToDelete(txn); - if (LOG.isLoggable(INFO)) LOG.info(ids.size() + " messages to delete"); - for (Entry e : ids.entrySet()) { - MessageId m = e.getKey(); - GroupId g = e.getValue(); - ClientMajorVersion cv = clientCache.get(g); - if (cv == null) { - Group group = db.getGroup(txn, g); - cv = new ClientMajorVersion(group.getClientId(), - group.getMajorVersion()); - clientCache.put(g, cv); + Map> ids = db.getMessagesToDelete(txn); + for (Entry> e : ids.entrySet()) { + GroupId groupId = e.getKey(); + Collection messageIds = e.getValue(); + if (LOG.isLoggable(INFO)) { + LOG.info(messageIds.size() + " messages to delete"); } + Group group = db.getGroup(txn, groupId); + ClientMajorVersion cv = new ClientMajorVersion(group.getClientId(), + group.getMajorVersion()); CleanupHook hook = hooks.get(cv); if (hook == null) { throw new IllegalStateException("No cleanup hook for " + cv); } - if (hook.deleteMessage(txn, g, m)) { - Collection messageIds = deleted.get(g); - if (messageIds == null) { - messageIds = new ArrayList<>(); - deleted.put(g, messageIds); - } - messageIds.add(m); - } else { - LOG.info("Message was not deleted"); - // Stop the timer so we don't keep trying to delete this message + for (MessageId m : messageIds) { + hook.deleteMessage(txn, groupId, m); db.stopCleanupTimer(txn, m); } - } - for (Entry> e : deleted.entrySet()) { - txn.attach(new MessagesCleanedUpEvent(e.getKey(), e.getValue())); + txn.attach(new MessagesCleanedUpEvent(groupId, messageIds)); } } 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 01056adb5..b6832d1e9 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 @@ -510,7 +510,8 @@ interface Database { *

* Read-only. */ - Map getMessagesToDelete(T txn) throws DbException; + Map> getMessagesToDelete(T txn) + throws DbException; /** * Returns the next time (in milliseconds since the Unix epoch) when a diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java index f3eae963c..9b7ac74fa 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java @@ -608,8 +608,8 @@ class DatabaseComponentImpl implements DatabaseComponent { } @Override - public Map getMessagesToDelete(Transaction transaction) - throws DbException { + public Map> getMessagesToDelete( + Transaction transaction) throws DbException { T txn = unbox(transaction); return db.getMessagesToDelete(txn); } 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 8a0ab01a9..72ceec594 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 @@ -2268,8 +2268,8 @@ abstract class JdbcDatabase implements Database { } @Override - public Map getMessagesToDelete(Connection txn) - throws DbException { + public Map> getMessagesToDelete( + Connection txn) throws DbException { long now = clock.currentTimeMillis(); PreparedStatement ps = null; ResultSet rs = null; @@ -2279,10 +2279,16 @@ abstract class JdbcDatabase implements Database { ps = txn.prepareStatement(sql); ps.setLong(1, now); rs = ps.executeQuery(); - Map ids = new HashMap<>(); + Map> ids = new HashMap<>(); while (rs.next()) { - ids.put(new MessageId(rs.getBytes(1)), - new GroupId(rs.getBytes(2))); + MessageId m = new MessageId(rs.getBytes(1)); + GroupId g = new GroupId(rs.getBytes(2)); + Collection messageIds = ids.get(g); + if (messageIds == null) { + messageIds = new ArrayList<>(); + ids.put(g, messageIds); + } + messageIds.add(m); } rs.close(); ps.close(); 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 d2780cc57..4dfec5fce 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 @@ -2474,14 +2474,14 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // When the timer expires, the message should be due and scheduled for // deletion time.set(now + duration); - assertEquals(singletonMap(messageId, groupId), + assertEquals(singletonMap(groupId, singletonList(messageId)), db.getMessagesToDelete(txn)); assertEquals(now + duration, db.getNextCleanupDeadline(txn)); // 1 ms after the timer expires, the message should be due and // scheduled for deletion time.set(now + duration + 1); - assertEquals(singletonMap(messageId, groupId), + assertEquals(singletonMap(groupId, singletonList(messageId)), db.getMessagesToDelete(txn)); assertEquals(now + duration, db.getNextCleanupDeadline(txn));