最近看京东分布式跟踪系统hydra代码,质量真的不高啊
public class GenerateTraceId {
private Long MAX_STEP = 0xffffffL;
private AtomicLong plusId = new AtomicLong(0L);
private long getPlusId() {
if (plusId.get() >= MAX_STEP) {
plusId.set(0L);
}
return plusId.incrementAndGet();
}
...
}
很容易猜测这个getplusid做什么的吧?就是顺序递增获得整数,特别的是,如果达到MAX_STEP就从0开始.
既然用了java.util.concurrent.atomic,肯定要线程安全,但是连续调用AtomicLong 的方法却没有任何保护措施,
难道作者以为系统会智能的帮你解决吗?
if (plusId.get() >= MAX_STEP) 这个成立后
plusId.set(0L); 这个执行时已经不能保证当前值是MAX_STEP了
正确写法:
return plusId.updateAndGet(new LongUnaryOperator(){
@Override
public long applyAsLong(long operand) {
return operand>=MAX_STEP?1:operand+1;
}
});
或者用lamda表达式:
private long getPlusId() {
return plusId.updateAndGet(i->i>=MAX_STEP?1:i+1);
}
呵呵,这个需要java8!不过也没关系看一下他的源码
public final long updateAndGet(IntUnaryOperator updateFunction) {
long prev, next;
do {
prev = get();
next = updateFunction.applyAsLong(prev);
} while (!compareAndSet(prev, next));
return next;
}
不用java8同样可以自己搞定
private long getPlusId() {
long currentid,nextid;
do{
currentid=plusId.get();
nextid=currentid>=MAX_STEP?1:currentid+1;
}while(!plusId.compareAndSet(currentid, nextid));
return nextid;
}
简单说,就是先读,然后计算新值,最后利用CAS(CompareAndSet)来设置新值,
如果没设置成功,说明get之后,已经被其他线程修改了,那么只要从新来一遍即可.
这样做的好处是不需要额外的锁,也可算一种乐观锁.