Sequence counter with persistence

Question

In one of my object I needed a sequence I would use to increment an Id field just like we often do with classical databases. As I have not found a suitable solution, I wrote one. This solution uses objectify.

The first approach I came with was to have a class with a static AtomicInteger field like this:

public class MySequence {
 static AtomicInteger sequence = new AtomicInteger(0);
}

I would then just call it each time I need it.

Problem:

Every time the server is restarted, the sequence is too. So we need to persist the last value.

Second approach:

Persist the last value with objectify:

 public class MySequence {
 private static AtomicInteger sequence = new AtomicInteger(0);
 private static List<PersistSequence> l = null;
private static PersistSequence newPs = null;
 /**
 * @param user_id the id of the user we want to get the last value of the sequence
 * @return last value of PersistSequence
 */
 public static int getNextValue( String user_id){
 if( sequence.get() == 0){//first time we run it or it has been restarted
 l = ofy().load().type( PersistSequence.class).filter("user_id", user_id).limit(1).list();//grabs PersistSequence object bounded to user_id
 if( l != null){
 int dataStoreSeq = -1;
 if ( l.size() > 0) dataStoreSeq = l.get(0).getLastId();
 if ( dataStoreSeq > sequence.get()) sequence.set( dataStoreSeq);
 persistCounter( user_id, sequence.incrementAndGet());
 return sequence.get();
 }else{
 persistCounter( user_id, sequence.incrementAndGet());
 return sequence.get();
 } 
 }else{
 persistCounter( user_id, sequence.incrementAndGet());
 return sequence.get();
 }
 }
 /**
 * @param user_id the id of the user
 * @param lastId last used Id
 */
 private static void persistCounter( String user_id, int lastId){
 //we only want one PersistSequence object per user so we delete the previous one before saving the new PersistSequence object
 List<Key<PersistSequence>> keys = ofy().load().type(PersistSequence.class).filter("user_id", user_id).keys().list();
 ofy().delete().keys(keys).now();
 //save the new one
 newPs = new PersistSequence();
 newPs.setLastId(lastId);
 newPs.setUserId(user_id);
 ofy().save().entities( newPs).now();
 } 
 }

I am happy with this code as it works well. However, I wonder if it still will rock if there is a high volume of users calling it.

Answer 1

Shouldn't get id twice

I'm concerned with this code:

 persistCounter( user_id, sequence.incrementAndGet());
 return sequence.get();

After you atomically increment and get the counter, you call sequence.get() again. That seems wrong because what happens if some other thread incremented the counter in the meantime? I think your code should be:

 int newId = sequence.incrementAndGet();
 persistCounter(user_id, newId);
 return newId;

Repeated code

Also, I noticed that the same block of code (the one above) appears three times in a row:

 public static int getNextValue( String user_id){
 if( sequence.get() == 0){//first time we run it or it has been restarted
 l = ofy().load().type(PersistSequence.class).
 filter("user_id", user_id).limit(1).list();
 if( l != null){
 int dataStoreSeq = -1;
 if ( l.size() > 0) dataStoreSeq = l.get(0).getLastId();
 if ( dataStoreSeq > sequence.get()) sequence.set( dataStoreSeq);
 persistCounter( user_id, sequence.incrementAndGet());
 return sequence.get();
 }else{
 persistCounter( user_id, sequence.incrementAndGet());
 return sequence.get();
 } 
 }else{
 persistCounter( user_id, sequence.incrementAndGet());
 return sequence.get();
 }
 }

Since that code happens for every case, you can simplify your function by extracting that common code and placing it right before the function returns:

 public static int getNextValue( String user_id){
 if( sequence.get() == 0){//first time we run it or it has been restarted
 l = ofy().load().type(PersistSequence.class).
 filter("user_id", user_id).limit(1).list();
 if( l != null){
 int dataStoreSeq = -1;
 if ( l.size() > 0) dataStoreSeq = l.get(0).getLastId();
 if ( dataStoreSeq > sequence.get()) sequence.set( dataStoreSeq);
 }
 }
 int newId = sequence.incrementAndGet();
 persistCounter(user_id, newId);
 return newId;
 }

Concurrency issues in persistCounter()

It's possible that persistCounter() will get called with out of order lastId arguments. For example, it's possible that you might get called with lastId of 5 and then later get called with a lastId of 4. So I think before you save the new counter, you should compare it with the old one to make sure that are actually saving a newer counter.

Also, there may be a concurrency problem with this function. What happens if one thread tries to save 4 and another tries to save 5 at the same time? Could it possibly end up saving 4 instead of 5?

• Arix OWAYE: You right with the code getNextValue method. I will correct it accordingly.
• Arix OWAYE: Also persisCounter() is private and it should only be called when the value of lastId has been retrieved. Please, see the code above. For concurrency issue I have added a call to ReentrantLock.