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