From c828db2e9562f9402cf494559713b6d1502e40ff Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Wed, 19 Oct 2011 16:34:58 +0100
Subject: [PATCH] Don't send subscription or transport updates unless an update
is due.
An update is due if the information has changed since the last update,
or if no update has been sent for 12 hours (to ensure that lost
updates are eventually replaced).
---
components/net/sf/briar/db/Database.java | 42 ++++++--
.../sf/briar/db/DatabaseComponentImpl.java | 78 ++++++++++-----
.../net/sf/briar/db/DatabaseConstants.java | 6 ++
components/net/sf/briar/db/JdbcDatabase.java | 99 ++++++++++++++++++-
.../sf/briar/db/DatabaseComponentTest.java | 72 +++++++++++++-
5 files changed, 256 insertions(+), 41 deletions(-)
diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java
index 90ec752112..f5dd6e39ae 100644
--- a/components/net/sf/briar/db/Database.java
+++ b/components/net/sf/briar/db/Database.java
@@ -344,6 +344,37 @@ interface Database<T> {
*/
Collection<Group> getSubscriptions(T txn, ContactId c) throws DbException;
+ /**
+ * Returns the time at which the subscriptions visible to the given contact
+ * were last modified.
+ * <p>
+ * Locking: contacts read, subscriptions read.
+ */
+ long getSubscriptionsModified(T txn, ContactId c) throws DbException;
+
+ /**
+ * Returns the time at which a subscription update was last sent to the
+ * given contact.
+ * <p>
+ * Locking: contacts read, subscriptions read.
+ */
+ long getSubscriptionsSent(T txn, ContactId c) throws DbException;
+
+ /**
+ * Returns the time at which the local transports were last modified.
+ * <p>
+ * Locking: transports read.
+ */
+ long getTransportsModified(T txn) throws DbException;
+
+ /**
+ * Returns the time at which a transport update was last sent to the given
+ * contact.
+ * <p>
+ * Locking: contacts read, transports read.
+ */
+ long getTransportsSent(T txn, ContactId c) throws DbException;
+
/**
* Returns the contacts to which the given group is visible.
* <p>
@@ -492,8 +523,8 @@ interface Database<T> {
* <p>
* Locking: contacts read, subscriptions write.
*/
- void setSubscriptionsModifiedTimestamp(T txn,
- Collection<ContactId> contacts, long timestamp) throws DbException;
+ void setSubscriptionsModified(T txn, Collection<ContactId> contacts,
+ long timestamp) throws DbException;
/**
* Records the time at which a subscription update was last sent to the
@@ -501,7 +532,7 @@ interface Database<T> {
* <p>
* Locking: contacts read, subscriptions write.
*/
- void setSubscriptionsSentTimestamp(T txn, ContactId c, long timestamp)
+ void setSubscriptionsSent(T txn, ContactId c, long timestamp)
throws DbException;
/**
@@ -520,8 +551,7 @@ interface Database<T> {
* <p>
* Locking: contacts read, transports write.
*/
- void setTransportsModifiedTimestamp(T txn, long timestamp)
- throws DbException;
+ void setTransportsModified(T txn, long timestamp) throws DbException;
/**
* Records the time at which a transport update was last sent to the given
@@ -529,7 +559,7 @@ interface Database<T> {
* <p>
* Locking: contacts read, transports write.
*/
- void setTransportsSentTimestamp(T txn, ContactId c, long timestamp)
+ void setTransportsSent(T txn, ContactId c, long timestamp)
throws DbException;
/**
diff --git a/components/net/sf/briar/db/DatabaseComponentImpl.java b/components/net/sf/briar/db/DatabaseComponentImpl.java
index 124974d91e..c89f788915 100644
--- a/components/net/sf/briar/db/DatabaseComponentImpl.java
+++ b/components/net/sf/briar/db/DatabaseComponentImpl.java
@@ -585,8 +585,8 @@ DatabaseCleaner.Callback {
public void generateSubscriptionUpdate(ContactId c, SubscriptionWriter s)
throws DbException, IOException {
- Map<Group, Long> subs;
- long timestamp;
+ Map<Group, Long> subs = null;
+ long timestamp = 0L;
contactLock.readLock().lock();
try {
if(!containsContact(c)) throw new NoSuchContactException();
@@ -594,9 +594,14 @@ DatabaseCleaner.Callback {
try {
T txn = db.startTransaction();
try {
- subs = db.getVisibleSubscriptions(txn, c);
- timestamp = System.currentTimeMillis();
- db.setSubscriptionsSentTimestamp(txn, c, timestamp);
+ // Work out whether an update is due
+ long modified = db.getSubscriptionsModified(txn, c);
+ long sent = db.getSubscriptionsSent(txn, c);
+ if(modified >= sent || updateIsDue(sent)) {
+ subs = db.getVisibleSubscriptions(txn, c);
+ timestamp = System.currentTimeMillis();
+ db.setSubscriptionsSent(txn, c, timestamp);
+ }
db.commitTransaction(txn);
} catch(DbException e) {
db.abortTransaction(txn);
@@ -608,15 +613,22 @@ DatabaseCleaner.Callback {
} finally {
contactLock.readLock().unlock();
}
- s.writeSubscriptions(subs, timestamp);
- if(LOG.isLoggable(Level.FINE))
- LOG.fine("Added " + subs.size() + " subscriptions to update");
+ if(subs != null) {
+ s.writeSubscriptions(subs, timestamp);
+ if(LOG.isLoggable(Level.FINE))
+ LOG.fine("Added " + subs.size() + " subscriptions to update");
+ }
+ }
+
+ private boolean updateIsDue(long sent) {
+ long now = System.currentTimeMillis();
+ return now - sent >= DatabaseConstants.MAX_UPDATE_INTERVAL;
}
public void generateTransportUpdate(ContactId c, TransportWriter t)
throws DbException, IOException {
- Map<TransportId, TransportProperties> transports;
- long timestamp;
+ Map<TransportId, TransportProperties> transports = null;
+ long timestamp = 0L;
contactLock.readLock().lock();
try {
if(!containsContact(c)) throw new NoSuchContactException();
@@ -624,9 +636,14 @@ DatabaseCleaner.Callback {
try {
T txn = db.startTransaction();
try {
- transports = db.getLocalTransports(txn);
- timestamp = System.currentTimeMillis();
- db.setTransportsSentTimestamp(txn, c, timestamp);
+ // Work out whether an update is due
+ long modified = db.getTransportsModified(txn);
+ long sent = db.getTransportsSent(txn, c);
+ if(modified >= sent || updateIsDue(sent)) {
+ transports = db.getLocalTransports(txn);
+ timestamp = System.currentTimeMillis();
+ db.setTransportsSent(txn, c, timestamp);
+ }
db.commitTransaction(txn);
} catch(DbException e) {
db.abortTransaction(txn);
@@ -638,9 +655,12 @@ DatabaseCleaner.Callback {
} finally {
contactLock.readLock().unlock();
}
- t.writeTransports(transports, timestamp);
- if(LOG.isLoggable(Level.FINE))
- LOG.fine("Added " + transports.size() + " transports to update");
+ if(transports != null) {
+ t.writeTransports(transports, timestamp);
+ if(LOG.isLoggable(Level.FINE))
+ LOG.fine("Added " + transports.size() +
+ " transports to update");
+ }
}
public TransportConfig getConfig(TransportId t) throws DbException {
@@ -1178,8 +1198,7 @@ DatabaseCleaner.Callback {
try {
if(!p.equals(db.getLocalProperties(txn, t))) {
db.setLocalProperties(txn, t, p);
- db.setTransportsModifiedTimestamp(txn,
- System.currentTimeMillis());
+ db.setTransportsModified(txn, System.currentTimeMillis());
changed = true;
}
db.commitTransaction(txn);
@@ -1290,20 +1309,31 @@ DatabaseCleaner.Callback {
public void setVisibility(GroupId g, Collection<ContactId> visible)
throws DbException {
- // Use HashSets for O(1) lookups, giving O(n) overall running time
- HashSet<ContactId> then, now;
+ Collection<ContactId> affected;
contactLock.readLock().lock();
try {
subscriptionLock.writeLock().lock();
try {
T txn = db.startTransaction();
try {
+ // Use HashSets for O(1) lookups, O(n) overall running time
+ HashSet<ContactId> then, now;
// Retrieve the group's current visibility
then = new HashSet<ContactId>(db.getVisibility(txn, g));
// Don't try to make the group visible to ex-contacts
now = new HashSet<ContactId>(visible);
now.retainAll(new HashSet<ContactId>(db.getContacts(txn)));
db.setVisibility(txn, g, now);
+ // Work out which contacts were affected by the change
+ affected = new ArrayList<ContactId>();
+ for(ContactId c : then) {
+ if(!now.contains(c)) affected.add(c);
+ }
+ for(ContactId c : now) {
+ if(!then.contains(c)) affected.add(c);
+ }
+ db.setSubscriptionsModified(txn, affected,
+ System.currentTimeMillis());
db.commitTransaction(txn);
} catch(DbException e) {
db.abortTransaction(txn);
@@ -1315,10 +1345,6 @@ DatabaseCleaner.Callback {
} finally {
contactLock.readLock().unlock();
}
- // Work out which contacts were affected by the change
- Collection<ContactId> affected = new ArrayList<ContactId>();
- for(ContactId c : then) if(!now.contains(c)) affected.add(c);
- for(ContactId c : now) if(!then.contains(c)) affected.add(c);
// Call the listeners outside the lock
if(!affected.isEmpty())
callListeners(new SubscriptionsUpdatedEvent(affected));
@@ -1344,7 +1370,6 @@ DatabaseCleaner.Callback {
public void unsubscribe(GroupId g) throws DbException {
if(LOG.isLoggable(Level.FINE)) LOG.fine("Unsubscribing from " + g);
- boolean removed = false;
Collection<ContactId> affected = null;
contactLock.readLock().lock();
try {
@@ -1359,7 +1384,6 @@ DatabaseCleaner.Callback {
if(db.containsSubscription(txn, g)) {
affected = db.getVisibility(txn, g);
db.removeSubscription(txn, g);
- removed = true;
}
db.commitTransaction(txn);
} catch(DbException e) {
@@ -1379,7 +1403,7 @@ DatabaseCleaner.Callback {
contactLock.readLock().unlock();
}
// Call the listeners outside the lock
- if(removed && !affected.isEmpty())
+ if(affected != null && !affected.isEmpty())
callListeners(new SubscriptionsUpdatedEvent(affected));
}
diff --git a/components/net/sf/briar/db/DatabaseConstants.java b/components/net/sf/briar/db/DatabaseConstants.java
index 737e97054f..fa013a562d 100644
--- a/components/net/sf/briar/db/DatabaseConstants.java
+++ b/components/net/sf/briar/db/DatabaseConstants.java
@@ -47,4 +47,10 @@ interface DatabaseConstants {
* recently sent batches have been acknowledged.
*/
static final int RETRANSMIT_THRESHOLD = 5;
+
+ /**
+ * The time in milliseconds after which a subscription or transport update
+ * should be sent to a contact even if no changes have occurred.
+ */
+ static final long MAX_UPDATE_INTERVAL = 12L * 60L * 60L * 1000L; // 12 hours
}
diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java
index 9565b54184..a6d6adc66d 100644
--- a/components/net/sf/briar/db/JdbcDatabase.java
+++ b/components/net/sf/briar/db/JdbcDatabase.java
@@ -1498,6 +1498,95 @@ abstract class JdbcDatabase implements Database<Connection> {
}
}
+ public long getSubscriptionsModified(Connection txn, ContactId c)
+ throws DbException {
+ PreparedStatement ps = null;
+ ResultSet rs = null;
+ try {
+ String sql = "SELECT modified FROM subscriptionTimestamps"
+ + " WHERE contactId = ?";
+ ps = txn.prepareStatement(sql);
+ ps.setInt(1, c.getInt());
+ rs = ps.executeQuery();
+ if(!rs.next()) throw new DbException();
+ long modified = rs.getLong(1);
+ if(rs.next()) throw new DbException();
+ rs.close();
+ ps.close();
+ return modified;
+ } catch(SQLException e) {
+ tryToClose(rs);
+ tryToClose(ps);
+ throw new DbException(e);
+ }
+ }
+
+ public long getSubscriptionsSent(Connection txn, ContactId c)
+ throws DbException {
+ PreparedStatement ps = null;
+ ResultSet rs = null;
+ try {
+ String sql = "SELECT sent FROM subscriptionTimestamps"
+ + " WHERE contactId = ?";
+ ps = txn.prepareStatement(sql);
+ ps.setInt(1, c.getInt());
+ rs = ps.executeQuery();
+ if(!rs.next()) throw new DbException();
+ long sent = rs.getLong(1);
+ if(rs.next()) throw new DbException();
+ rs.close();
+ ps.close();
+ return sent;
+ } catch(SQLException e) {
+ tryToClose(rs);
+ tryToClose(ps);
+ throw new DbException(e);
+ }
+ }
+
+ public long getTransportsModified(Connection txn) throws DbException {
+ PreparedStatement ps = null;
+ ResultSet rs = null;
+ try {
+ String sql = "SELECT DISTINCT modified FROM transportTimestamps";
+ ps = txn.prepareStatement(sql);
+ rs = ps.executeQuery();
+ if(!rs.next()) throw new DbException();
+ long modified = rs.getLong(1);
+ if(rs.next()) throw new DbException();
+ rs.close();
+ ps.close();
+ return modified;
+ } catch(SQLException e) {
+ tryToClose(rs);
+ tryToClose(ps);
+ throw new DbException(e);
+ }
+ }
+
+ public long getTransportsSent(Connection txn, ContactId c)
+ throws DbException {
+ PreparedStatement ps = null;
+ ResultSet rs = null;
+ try {
+ String sql = "SELECT sent FROM transportTimestamps"
+ + " WHERE contactId = ?";
+ ps = txn.prepareStatement(sql);
+ ps.setInt(1, c.getInt());
+ rs = ps.executeQuery();
+ if(!rs.next()) throw new DbException();
+ long sent = rs.getLong(1);
+ if(rs.next()) throw new DbException();
+ rs.close();
+ ps.close();
+ return sent;
+ } catch(SQLException e) {
+ tryToClose(rs);
+ tryToClose(ps);
+ throw new DbException(e);
+ }
+ }
+
public Collection<ContactId> getVisibility(Connection txn, GroupId g)
throws DbException {
PreparedStatement ps = null;
@@ -2112,7 +2201,7 @@ abstract class JdbcDatabase implements Database<Connection> {
}
}
- public void setSubscriptionsModifiedTimestamp(Connection txn,
+ public void setSubscriptionsModified(Connection txn,
Collection<ContactId> contacts, long timestamp) throws DbException {
PreparedStatement ps = null;
try {
@@ -2137,7 +2226,7 @@ abstract class JdbcDatabase implements Database<Connection> {
}
}
- public void setSubscriptionsSentTimestamp(Connection txn, ContactId c,
+ public void setSubscriptionsSent(Connection txn, ContactId c,
long timestamp) throws DbException {
PreparedStatement ps = null;
try {
@@ -2219,7 +2308,7 @@ abstract class JdbcDatabase implements Database<Connection> {
}
}
- public void setTransportsModifiedTimestamp(Connection txn, long timestamp)
+ public void setTransportsModified(Connection txn, long timestamp)
throws DbException {
PreparedStatement ps = null;
try {
@@ -2234,8 +2323,8 @@ abstract class JdbcDatabase implements Database<Connection> {
}
}
- public void setTransportsSentTimestamp(Connection txn, ContactId c,
- long timestamp) throws DbException {
+ public void setTransportsSent(Connection txn, ContactId c, long timestamp)
+ throws DbException {
PreparedStatement ps = null;
try {
String sql = "UPDATE transportTimestamps SET sent = ?"
diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java
index 65c0acea60..fecd3f4183 100644
--- a/test/net/sf/briar/db/DatabaseComponentTest.java
+++ b/test/net/sf/briar/db/DatabaseComponentTest.java
@@ -735,6 +735,34 @@ public abstract class DatabaseComponentTest extends TestCase {
context.assertIsSatisfied();
}
+ @Test
+ public void testSubscriptionUpdateNotSentUnlessDue() throws Exception {
+ final long now = System.currentTimeMillis();
+ Mockery context = new Mockery();
+ @SuppressWarnings("unchecked")
+ final Database<Object> database = context.mock(Database.class);
+ final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+ final SubscriptionWriter subscriptionWriter =
+ context.mock(SubscriptionWriter.class);
+ context.checking(new Expectations() {{
+ allowing(database).startTransaction();
+ will(returnValue(txn));
+ allowing(database).commitTransaction(txn);
+ allowing(database).containsContact(txn, contactId);
+ will(returnValue(true));
+ // Check whether an update is due
+ oneOf(database).getSubscriptionsModified(txn, contactId);
+ will(returnValue(now - 1L));
+ oneOf(database).getSubscriptionsSent(txn, contactId);
+ will(returnValue(now));
+ }});
+ DatabaseComponent db = createDatabaseComponent(database, cleaner);
+
+ db.generateSubscriptionUpdate(contactId, subscriptionWriter);
+
+ context.assertIsSatisfied();
+ }
+
@Test
public void testGenerateSubscriptionUpdate() throws Exception {
final MessageId messageId1 = new MessageId(TestUtils.getRandomId());
@@ -753,10 +781,15 @@ public abstract class DatabaseComponentTest extends TestCase {
allowing(database).commitTransaction(txn);
allowing(database).containsContact(txn, contactId);
will(returnValue(true));
+ // Check whether an update is due
+ oneOf(database).getSubscriptionsModified(txn, contactId);
+ will(returnValue(0L));
+ oneOf(database).getSubscriptionsSent(txn, contactId);
+ will(returnValue(0L));
// Get the visible subscriptions
oneOf(database).getVisibleSubscriptions(txn, contactId);
will(returnValue(Collections.singletonMap(group, 0L)));
- oneOf(database).setSubscriptionsSentTimestamp(with(txn), with(contactId),
+ oneOf(database).setSubscriptionsSent(with(txn), with(contactId),
with(any(long.class)));
// Add the subscriptions to the writer
oneOf(subscriptionWriter).writeSubscriptions(
@@ -770,6 +803,34 @@ public abstract class DatabaseComponentTest extends TestCase {
context.assertIsSatisfied();
}
+ @Test
+ public void testTransportUpdateNotSentUnlessDue() throws Exception {
+ final long now = System.currentTimeMillis();
+ Mockery context = new Mockery();
+ @SuppressWarnings("unchecked")
+ final Database<Object> database = context.mock(Database.class);
+ final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+ final TransportWriter transportWriter =
+ context.mock(TransportWriter.class);
+ context.checking(new Expectations() {{
+ allowing(database).startTransaction();
+ will(returnValue(txn));
+ allowing(database).commitTransaction(txn);
+ allowing(database).containsContact(txn, contactId);
+ will(returnValue(true));
+ // Check whether an update is due
+ oneOf(database).getTransportsModified(txn);
+ will(returnValue(now - 1L));
+ oneOf(database).getTransportsSent(txn, contactId);
+ will(returnValue(now));
+ }});
+ DatabaseComponent db = createDatabaseComponent(database, cleaner);
+
+ db.generateTransportUpdate(contactId, transportWriter);
+
+ context.assertIsSatisfied();
+ }
+
@Test
public void testGenerateTransportUpdate() throws Exception {
final MessageId messageId1 = new MessageId(TestUtils.getRandomId());
@@ -788,10 +849,15 @@ public abstract class DatabaseComponentTest extends TestCase {
allowing(database).commitTransaction(txn);
allowing(database).containsContact(txn, contactId);
will(returnValue(true));
+ // Check whether an update is due
+ oneOf(database).getTransportsModified(txn);
+ will(returnValue(0L));
+ oneOf(database).getTransportsSent(txn, contactId);
+ will(returnValue(0L));
// Get the local transport properties
oneOf(database).getLocalTransports(txn);
will(returnValue(transports));
- oneOf(database).setTransportsSentTimestamp(with(txn), with(contactId),
+ oneOf(database).setTransportsSent(with(txn), with(contactId),
with(any(long.class)));
// Add the properties to the writer
oneOf(transportWriter).writeTransports(with(transports),
@@ -1287,7 +1353,7 @@ public abstract class DatabaseComponentTest extends TestCase {
oneOf(database).getLocalProperties(txn, transportId);
will(returnValue(new TransportProperties()));
oneOf(database).setLocalProperties(txn, transportId, properties);
- oneOf(database).setTransportsModifiedTimestamp(with(txn),
+ oneOf(database).setTransportsModified(with(txn),
with(any(long.class)));
oneOf(database).commitTransaction(txn);
oneOf(listener).eventOccurred(with(any(
--
GitLab