JdbcStore concurrency bug fix#1100
Conversation
|
hi @the-thing cool, thanks. So your stress test with your changes passes now, I assume. :) |
|
Yes it does - you should run it. Obviously this relies still on incoming/outgoing sequences being guarded by the appropriate locks, but at least one sequence update will not pollute the other. |
|
|
Have a look if it looks any better. |
|
I got this: (I stopped execution after some time) |
|
Which Java version did you use to run? And also, stupid to ask, but did you pull the branch changes? |
Nevermind, I cloned your fork but forgot to switch to the specific branch. ;) Looking better now, thanks. :) |
chrjohn
left a comment
There was a problem hiding this comment.
I meant to ask you yesterday about this when you introduced the IntSupplier... what was the reason for reverting it now?
I thought the reason for introducing it was because the value was obtained when it was used in storeSequenceNumber(). Now it is already fetched in setNextSender/TargetSeqNum().
Probably makes no difference, but just wanted to ask anyway. ;)
Also, I am asking myself if we can't just take the value of next right away? In theory there could be an increment by another thread between cache.setNext... and storeSequenceNumber() with cache.getNext... as parameter. But that's what you probably meant by #1100 (comment)
|
I think it is just simpler. Lambda is lazy is it usually better if there is a chance that something will fail or return faster before lambda gets called. (in this case it doesn't matter and shouldn't be there) You are right about the next value. I will fix it. |
Fixes #357
Changes