I think the recipe could be improved by having an alternate trySetValue that takes a version. So, you get the current value AND version, then trySetValue with both the value and expected version.
-JZ On October 1, 2014 at 4:43:00 PM, Jordan Zimmerman ([email protected]) wrote: That’s fair. I don’t remember how this recipe came to be. I think someone at Netflix needed it. -JZ On October 1, 2014 at 4:38:51 PM, Scott Blum ([email protected]) wrote: I think, on further reflection, what bothers me is that the class as a whole appears to be misleading. From the user's point of view, there's no actual way to achieve optimistic locking. The method description on trySetValue() states "Changes the shared value only if its value has not changed since this client last read it" -- but this is a meaningless statement from the point of view of the user, because the user has no visibility into SharedValue's internal state and no way to make any kind of correlation between the user's last read and sharedValue's last read. The most you can say is that it's a best-effort value cache with no user-facing lock semantics at all. I would suggest a serious update to the class's doc, perhaps linking to the DistributedAtomic equivalents. On Wed, Oct 1, 2014 at 5:23 PM, Jordan Zimmerman <[email protected]> wrote: On October 1, 2014 at 4:18:52 PM, Scott Blum ([email protected]) wrote: 4) Meanwhile user computes A -> A'. 5) User calls sharedValue.trySetValue(A'). This succeeds because sharedValue thinks it's up to date; it has no idea that the user is setting a value based on old data. Well, I’d call that a user error. SharedValue is not an atomic value (you found that recipe already). It’s meant to get updated in the background as a kind of cache. It can be thought of as a cached node that has optimistic locking semantics for updating. -JZ
