Hi, On 22/08/14 16:31, "Alex Parvulescu" <[email protected]> wrote: >Following OAK-2039 there was a discussion around the current design of the >#checkpoint apis. [0] > >It looks a bit confusing that you can call the apis to create a checkpoint >and get back a reference but when retrieving it, it might not exist, even >if the calls are back to back. >With OAK-2039 I've added some warning logs when a checkpoint cannot be >created but a ref is still returned, to understand if this is a system >load >problem, or something more profound.
what is the reason the SegmentNodeStore does a commitSemaphore.tryAcquire() instead of a commitSemaphore.acquire() like in SegmentNodeStore.merge()? >I believe that nobody has any issues with the #retrieve method, all the >confusion is really about the #checkpoint parts, currently marked as >'@Nonnull'. > >Alternatives mentioned are > - return null if the checkpoint was not created > - throw en exception > >I vote -0 for the change, I believe that making this more complicated than >it needs to be (more null checks, or a try/catch) doesn't really benefit >anybody. I think we should improve it somehow because I find the current behaviour quite confusing. The current implementation of SegmentNodeStore.checkpoint() IMO violates the contract. It may return a string reference to a checkpoint which was never created and obviously won't be valid for the requested lifetime. In my view, a client should be able to detect this in a simple way. Right now you would have to call retrieve() to find out if checkpoint() actually worked. Returning a null value works better if we specify under what conditions no checkpoint can be created. After all a client would have to implement some code in response to a null value. E.g. should it retry later, because the checkpoint cannot be created when the system is under load? This would be a good fit if we keep the current implementation in SegmentNodeStore. An exception works better if we say an implementation should always be able to create a checkpoint and only fail if it cannot perform the operation because of e.g. an underlying IOException. Regards Marcel
