Hi,

I found a possible data race on uid generation, in Cassandra's
james-mailbox subproject.

The problem appears when nextUid is called for the first time ( the last
generated uid is equal to 0 ).

The previous code was performing a select on the last uid value.
 - If this value is null, it updates it without checking it.
 - If the value is not null, it updates the value, with a lightweight
transaction.



I solved the issue by :
 - creating the uid value if the uid is equal to 0. If we can perform
this operation, no one can do, and we can use 1 as next value for uid.
If we fail, someone did that, and we have to update the last uid value
using lightweight transactions ( as if last uid was not null ).
 - I also added a max retry parameter : if we try too update the value
of the uid too many times, a Mailbox exception will be thrown.

You will find the corresponding patch as an attachment.

Sincerly yours,

Benoit Tellier
diff -uNr james-mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraUidProvider.java james-mailbox-new/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraUidProvider.java
--- james-mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraUidProvider.java	2014-12-17 16:04:47.600086583 +0100
+++ james-mailbox-new/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraUidProvider.java	2014-12-17 16:14:10.816777612 +0100
@@ -19,10 +19,11 @@
 
 package org.apache.james.mailbox.cassandra.mail;
 
+import static com.datastax.driver.core.querybuilder.QueryBuilder.insertInto;
+import static com.datastax.driver.core.querybuilder.QueryBuilder.update;
 import static com.datastax.driver.core.querybuilder.QueryBuilder.eq;
-import static com.datastax.driver.core.querybuilder.QueryBuilder.select;
 import static com.datastax.driver.core.querybuilder.QueryBuilder.set;
-import static com.datastax.driver.core.querybuilder.QueryBuilder.update;
+import static com.datastax.driver.core.querybuilder.QueryBuilder.select;
 import static org.apache.james.mailbox.cassandra.table.CassandraMessageUidTable.MAILBOX_ID;
 import static org.apache.james.mailbox.cassandra.table.CassandraMessageUidTable.NEXT_UID;
 import static org.apache.james.mailbox.cassandra.table.CassandraMessageUidTable.TABLE_NAME;
@@ -38,24 +39,40 @@
 import com.datastax.driver.core.Session;
 
 public class CassandraUidProvider implements UidProvider<UUID> {
+    public final static int DEFAULT_MAX_RETRY = 100000;
+
     private Session session;
     private final int applied = 0;
+    private int maxRetry;
 
-    public CassandraUidProvider(Session session) {
+    public CassandraUidProvider(Session session, int maxRetry) {
         this.session = session;
+        this.maxRetry = maxRetry;
+    }
+
+    public CassandraUidProvider(Session session) {
+        this(session, DEFAULT_MAX_RETRY);
     }
 
     @Override
     public long nextUid(MailboxSession mailboxSession, Mailbox<UUID> mailbox) throws MailboxException {
-        ResultSet resultat = null;
         long lastUid = lastUid(mailboxSession, mailbox);
         if (lastUid == 0) {
-            resultat = session.execute(update(TABLE_NAME).with(set(NEXT_UID, ++lastUid)).where(eq(MAILBOX_ID, mailbox.getMailboxId())));
-        } else {
-            do {
-                lastUid = lastUid(mailboxSession, mailbox);
-                resultat = session.execute(update(TABLE_NAME).onlyIf(eq(NEXT_UID, lastUid)).with(set(NEXT_UID, ++lastUid)).where(eq(MAILBOX_ID, mailbox.getMailboxId())));
-            } while (!resultat.one().getBool(applied));
+            ResultSet result = session.execute(insertInto(TABLE_NAME).value(NEXT_UID, ++lastUid).value(MAILBOX_ID, mailbox.getMailboxId()).ifNotExists());
+            if(result.one().getBool(applied)) {
+                return lastUid;
+            }
+        }
+        int tries = 0;
+        boolean isApplied;
+        do {
+            tries++;
+            lastUid = lastUid(mailboxSession, mailbox);
+            ResultSet result = session.execute(update(TABLE_NAME).onlyIf(eq(NEXT_UID, lastUid)).with(set(NEXT_UID, ++lastUid)).where(eq(MAILBOX_ID, mailbox.getMailboxId())));
+            isApplied = result.one().getBool(applied);
+        } while (! isApplied && tries < maxRetry);
+        if( ! isApplied ) {
+            throw new MailboxException("Can not obtain rights to manage UID");
         }
         return lastUid;
     }

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to