curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Drob <mad...@cloudera.com>
Subject Re: Exception throwing
Date Fri, 01 Aug 2014 22:04:18 GMT
The set with version is basically a compareAndSet. java.util chooses to
implement this also with a 'return false' value for when some other thread
got there first. I'll have to go back and look at the Curator API and see
how these failures are currently communicated.


On Fri, Aug 1, 2014 at 4:08 PM, John Vines <vines@apache.org> wrote:

> There's KeeperException.BADVERSION, which is when you
> setData().withVersion and the version in ZK changed vs. the version seen
> prior. That one is pretty critical to support, IMO. The other cases around
> node existance can definitely be handled by the user, but given the
> possibilities for races in distributed systems you still can't be certain.
> But there could be user cases where they want to create a node and not fail
> if it exists or have a delete pass if the node was already deleted by
> something else (not sure if a flag was added for that case, I vaguely
> recall a discussion).
>
>
> On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <madrob@cloudera.com> wrote:
>
>> John, thanks for your input. So some of this is improper use of the API,
>> right? If you are attempting to create a node and it already exists, then
>> that can be an exceptional case. If you just want to make sure that a node
>> exists, regardless of who created it, then that's a case for a different
>> API. Asserting that you created the node could be important - see the
>> distinction in ConcurrentMap between put() and putIfAbsent(). Then again,
>> failing to create the node because it already exists might not need to be
>> an exceptional case and simply warrants a 'return false' on the method? Do
>> the other cases you mentioned have similar analogues? Maybe the end result
>> of what comes out of this is better docs.
>>
>> Mike
>>
>>
>> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vines@apache.org> wrote:
>>
>>> It's not a matter of it being a bug, it's a matter of usability. Because
>>> every single method just throws Exception it gives me, as a user,
>>> absolutely zero inclination at writing time to figure out what sort of
>>> failures can happen. And the complete lack of javadocs compound this
>>> issue.
>>> This has been my biggest issue with Curator.
>>>
>>> Yes, there are some unrecoverable errors. But not all of them are, such
>>> as
>>> a subset of the KeeperExceptions around node state, security, and
>>> versions.
>>> I could be sold on a split, where those type of items are exposed and
>>> then
>>> the critical ones you keep mentioning are Runtime. But as much as I
>>> dislike
>>> generic Exceptions for everything, forcing users to catch
>>> RuntimeExceptions
>>> to do proper exception handling for known and well defined exceptions is
>>> an
>>> awful practice to put people though.
>>>
>>>
>>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
>>> jordan@jordanzimmerman.com
>>> > wrote:
>>>
>>> > -1 (binding)
>>> >
>>> > If I could go back I’d remove all checked exceptions entirely. I don’t
>>> > think there’s an objective answer here - it comes down to personal
>>> > preference, etc. I don’t see much value in touching nearly every file
>>> in
>>> > the library in order to do this. We’ve had maybe 2 or 3 requests in the
>>> > many years that Curator has exists. This suggests that the overwhelming
>>> > majority accept the current exception semantics. If you can point to an
>>> > actual bug that this causes that would be helpful.
>>> >
>>> > -Jordan
>>> >
>>> > From: Mike Drob <madrob@cloudera.com>
>>> > Reply: dev@curator.apache.org <dev@curator.apache.org>>
>>> > Date: August 1, 2014 at 2:32:07 PM
>>> > To: dev@curator.apache.org <dev@curator.apache.org>>
>>> > Subject:  Exception throwing
>>> >
>>> > I'd like to revisit the discussion around always throwing Exception in
>>> the
>>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that
>>> touch
>>> > on this subject, but I think there is a good conversation to be had.
>>> >
>>> > I understand the suggestions that if an exception is thrown, we are in
>>> a
>>> > bad state, regardless of the type of exception. However, throwing
>>> Exception
>>> > comes with some unfortunate Java baggage...
>>> >
>>> > By declaring thrown Exception, we force consumers to also catch
>>> > RuntimeExceptions instead of letting them propagate as they normally
>>> would.
>>> > In some cases, the calling code may need to attempt roll-back
>>> semantics, or
>>> > retry outside of what Curator provides, or something else that we
>>> haven't
>>> > thought of.
>>> >
>>> > I'd like to propose replacing much of the thrown Exception methods with
>>> > CuratorException. This will still carry the connotation that it doesn't
>>> > matter what kind of exception we encounter, they're all bad. It will
>>> also
>>> > be backwards compatible with the previous API, since users will still
>>> be
>>> > able to catch Exception in their calling code. And it has the
>>> advantage of
>>> > separating checked exceptions from unchecked ones, so that users don't
>>> > unintentionally catch something unrelated.
>>> >
>>> > Thoughts?
>>> >
>>> > I tried looking for more details behind the design decision to always
>>> throw
>>> > Exception, but wasn't able to find them. If they're already
>>> documented, I'd
>>> > love to be pointed at the wiki or site, or a mailing list thread will
>>> do in
>>> > a pinch.
>>> >
>>> > Thanks,
>>> > Mike
>>> >
>>>
>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message