kezhuw commented on PR #478:
URL: https://github.com/apache/curator/pull/478#issuecomment-1704450591

   @Hexiaoqiao Thanks your sharing, that is important!
   
   I found some potential problem in the usages, though I haven't take a deep 
in look. `incrSharedCount` uses batch which is what Ted Dunning suggested in 
above mail thread. That is good. But it is somewhat error-prone in 
implementation.
   1. The counter itself is a 32-bit integer, so it will overflow finally 
regard less of `Stat.version`.
   2. The default `batchSize` is small. I got two defaults. `public static 
final int DEFAULT_SEQ_NUM_BATCH_SIZE = 10;` and `public static final int 
ZK_DTSM_TOKEN_SEQNUM_BATCH_SIZE_DEFAULT = 1;`.
   
   So, for your use case, I suggest:
   1. Rewrites sequence number as 64-bit integer. I am not sure what the 
consequence of this. But 32-bit in your case will overflow finally. I could 
overflowed already hiding by overflow of `Stat.version`.
   2. Uses a sensible default batch size and refuse small batch size. This is 
easy in your case, but you will get negative sequence number cause of above.
   
   `Stat.version` is 32-bit, it is not good to do a 1 to 1 mapping to 
application sequence number.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to