Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
Accumulo >> mail # dev >> Review Request 15166: ACCUMULO-802 Tablespaces


+
Christopher Tubbs 2013-11-01, 02:01
+
John Vines 2013-11-01, 22:19
+
Christopher Tubbs 2013-11-08, 01:08
+
John Vines 2013-11-08, 19:58
+
Sean Hickey 2013-11-10, 00:50
+
Christopher Tubbs 2013-11-14, 21:40
+
Christopher Tubbs 2013-11-14, 21:07
+
Christopher Tubbs 2013-11-18, 22:09
+
John Vines 2013-11-29, 18:59
+
Christopher Tubbs 2013-12-04, 21:44
+
Christopher Tubbs 2013-12-04, 21:41
+
John Vines 2013-12-04, 22:31
+
Christopher Tubbs 2013-12-04, 21:26
+
Christopher Tubbs 2013-12-04, 21:37
+
keith@... 2013-11-08, 19:51
+
Keith Turner 2013-11-08, 19:58
+
Keith Turner 2013-11-08, 23:10
+
Sean Hickey 2013-11-10, 01:16
+
Christopher Tubbs 2013-12-04, 21:54
+
Christopher Tubbs 2013-12-04, 23:44
+
Mike Drob 2013-11-08, 19:08
+
Mike Drob 2013-11-08, 19:27
+
John Vines 2013-11-08, 19:58
+
Christopher Tubbs 2013-11-08, 20:10
+
Josh Elser 2013-11-08, 22:16
+
John Vines 2013-11-08, 22:01
+
John Vines 2013-11-08, 21:55
+
Mike Drob 2013-11-08, 21:52
Copy link to this message
-
Re: Review Request 15166: ACCUMULO-802 Tablespaces


> On Nov. 8, 2013, 7:08 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/TableNamespaceExistsException.java, line 24
> > <https://reviews.apache.org/r/15166/diff/1/?file=376077#file376077line24>
> >
> >     I would like to see this (and the other two exceptions) all extend RuntimeException. I think that since these are all conditions that can be checked for before attempting operations, it is unnecessarily burdensome on the client to force additional checked Exceptions on them.
>
> John Vines wrote:
>     A million times no. Do NOT force clients to catch RuntimeExceptions to handle your error codes.
>
> Christopher Tubbs wrote:
>     I agree they should not be RuntimeExceptions.

You're already forcing clients to catch the exceptions when you declare them as checked exceptions. And there is an exists method on operations, which is completely pointless with the checked exceptions. Consider the following code snippets:

// Can write this if TableNamespaceExistsException is Runtime
if (! namespaceOperations.exists("foo"))
  namespaceOperations.create("foo")

// If it is checked, then you have to write
if (! namespaceOperations.exists("foo")) {
  try {
    namespaceOperations.create("foo")
  } catch (TableNamespaceExistsException tnee) {
    // log? nothing we can do anyway.
  }
}

// Or even worse, skip the existence check entirely and just try to create...
// With this sample, what is even the point of having an exists() method?
try {
  namespaceOperations.create("foo")
} catch (Exception e) {
  // log? nothing we can do anyway.
}
Now, my preferred snippet (#1) _is_ susceptible to a potential race condition, but I think the solution is a more serious API redesign. Maybe have create return a boolean instead of throwing, similar to Collection.add(). Or maybe add a createIfAbsent() method, similar to ConcurrentHashMap.putIfAbsent().

There's lots of ways to do this, but forcing users to catch clunky exceptions is one of my least favorite parts of Java. Also see Effective Java, Item 59.
> On Nov. 8, 2013, 7:08 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperationsImpl.java, line 413
> > <https://reviews.apache.org/r/15166/diff/1/?file=376084#file376084line413>
> >
> >     If an error happens with a table in the middle of the list, then we're left in a half-cloned state. Probably better to clean up after ourselves.
>
> Christopher Tubbs wrote:
>     I'm thinking we should just drop the ability to clone namespaces. It creates too many issues to watch for, and it isn't that hard to do in client code.

+1
> On Nov. 8, 2013, 7:08 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperationsImpl.java, line 555
> > <https://reviews.apache.org/r/15166/diff/1/?file=376084#file376084line555>
> >
> >     What happens if you attempt to offline a namespace while somebody else moves a table from one namespace into another? What _should_ happen?
>
> Christopher Tubbs wrote:
>     Good question. I'm thinking we should drop the ability to migrate tables between namespaces. It creates too many concurrency issues. Besides, it doesn't buy us much if we can clone into a new namespace.

+1
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15166/#review28554
-----------------------------------------------------------
On Nov. 1, 2013, 2:01 a.m., Christopher Tubbs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15166/
> -----------------------------------------------------------
>
> (Updated Nov. 1, 2013, 2:01 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-802
>     https://issues.apache.org/jira/browse/ACCUMULO-802
>
>
> Repository: accumulo
>
>
> Description
> -------
+
Sean Hickey 2013-11-10, 00:57
+
Christopher Tubbs 2013-11-18, 22:13
+
John Vines 2013-11-18, 22:00
+
Christopher Tubbs 2013-11-18, 21:45
+
Christopher Tubbs 2013-11-18, 21:29
+
keith@... 2013-11-08, 20:11