Hi, I think we need to decide whether "not beeing able to create checkpoint" is an important and common case or not.
* If it's an common case (if it uses tryAcquire which may or may not work), then I would prefer "tryCheckpoint" with a possible return value of null. * If it's not a common case, then throwing an exception would be fine. Or maybe we need both "tryCheckpoint" and "checkpoint"? But using "checkpoint" as the method name, and using a "special return value" if it didn't work, that's dangerous, as it's so easy (specially for new developers) to forget to check the return value. The Java API has some similarly dangerous methods, for example InputStream.read(byte[] b...) which may not read fully, that's an example on what to avoid I think. Regards, Thomas On 27/08/14 18:09, "Jukka Zitting" <[email protected]> wrote: >Hi, > >2014-08-22 10:31 GMT-04:00 Alex Parvulescu <[email protected]>: >> 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. > >I'd call that a bug, as the behavior clearly breaks the documented >contract of the method. > >> Alternatives mentioned are >> - return null if the checkpoint was not created >> - throw en exception > >Throwing an exception seems like the correct solution here, as not >being able to create a checkpoint is an exceptional situation, much >like when a commit fails for similar reasons. > >> 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 there's more extra complication in having to deal with a >potentially non-existent return value. > >BR. > >Jukka Zitting
