The class implementation below wraps two AtomicInteger fields. After the max=9999 value is reached, the main sequence number is expected to reset to zero. When the sequence number resets, the resetCount should be increased by one.
This i think is the critical path.
The goal is to ensure unique sequenced IDs returned to highly concurrent callers.
public class SequenceGenerator {
private static final SequenceGenerator GENERATOR = new SequenceGenerator();
private final AtomicInteger sequence = new AtomicInteger(0);
private final AtomicInteger resetCount = new AtomicInteger(0);
private SequenceGenerator() {}
public static SequenceGenerator getInstance() {
return GENERATOR;
}
/**
* This method a unique sequenceId that follow this format
* [ResetCount-SequenceNumber]. The SequenceNumber range is [0000,9999]
* and every time the max value 9999 exceeds, it will reset back to zero
* and the ResetCount value will increase by one.
*
* @return a unique sequenceId in literal format
*/
public String generateSequenceId() {
while (true) {
int local = sequence.getAndIncrement();
if (local <= 9999) {
//Do some work and return a unique sequence number literal
return resetCount+"-"+local;
} else {
//Reset after the sequence exceeds 4 digits
//Only a thread with variable local=10000 should
//reset sequence back to zero(0)
if(local == 10000){
sequence.set(0);
resetCount.incrementAndGet();
}
}
}
}
}
Updates
Based on the discussion and answer below, the take is that compound AtomicInteger operations(more than one) are not thread-safe. Credit to @Solomon Slow
I updated the method above, and hopefully, it is according to the specs and thread-safe
private final AtomicInteger sequence = new AtomicInteger(0);
private final AtomicInteger resetCount = new AtomicInteger(0);
private final Object resetLock = new Object();
public String generateSequenceId() {
while (true) {
int local = sequence.getAndIncrement();
if (local <= 9999) {
return resetCount+"-"+local;
} else {
//Reset after the sequence exceeds 4 digits
//Only a thread with variable local=10000 should
//reset sequence back to zero(0)
synchronized(resetLock){
if(sequence.get()>=10000){
sequence.set(0);
resetCount.incrementAndGet();
}
}
}
}
}
In the latest code update, the suggestion is that, the sequence number reset and reset count increment is inside a synchronized block. Only one thread triggers the sequence reset and increases the reset count.
Although, I am not sure if there will be an issue between sequence.set(0) and sequence.getAndIncrement() calls if they happen at the same time called by different threads.
sequence.set(0);andresetCount.incrementAndGet()should be swapped, with the current code some other thread can see the 0 sequence, but with the old reset count.