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;
}
signature.asc
Description: OpenPGP digital signature
