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

Switch to Threaded View
Accumulo, mail # dev - Review Request 15166: ACCUMULO-802 Tablespaces


Copy link to this message
-
Re: Review Request 15166: ACCUMULO-802 Tablespaces
Christopher Tubbs 2013-11-08, 20:10


> On Nov. 8, 2013, 2: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.

I agree they should not be RuntimeExceptions.
> On Nov. 8, 2013, 2:08 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperations.java, line 235
> > <https://reviews.apache.org/r/15166/diff/1/?file=376080#file376080line235>
> >
> >     Please do not introduce whitespace errors.

I'll reformat all the edited files before merging.
> On Nov. 8, 2013, 2:08 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperations.java, line 40
> > <https://reviews.apache.org/r/15166/diff/1/?file=376082#file376082line40>
> >
> >     Should TableOperations and TabletOpoerations share a common super-interface?

Can improve that later.
> On Nov. 8, 2013, 2:08 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/admin/TableNamespaceOperations.java, line 84
> > <https://reviews.apache.org/r/15166/diff/1/?file=376082#file376082line84>
> >
> >     I don't understand what this method does. Sets the default existance of versioning iterator on tables created in the namespace? That seems like maybe too broad of a setting to expose through this API?

I think you're right. We don't really need to have convenience methods to set up iterators by default on namespaces, especially since we already have them on creating tables. Besides, this is a trivial matter of: 1) create namespace, 2) configure namespace, 3) create table
> On Nov. 8, 2013, 2: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.

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.
> On Nov. 8, 2013, 2: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?

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.
> On Nov. 8, 2013, 2:08 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/TableNamespaces.java, line 42
> > <https://reviews.apache.org/r/15166/diff/1/?file=376087#file376087line42>
> >
> >     Is there value in making this a SortedMap? Why not just Map?

Sorted is always nice... the number of namespaces should be small enough to not really matter.
> On Nov. 8, 2013, 2:08 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java, line 25
> > <https://reviews.apache.org/r/15166/diff/1/?file=376088#file376088line25>
> >
> >     No.

Bug!
- Christopher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15166/#review28554
On Oct. 31, 2013, 10:01 p.m., Christopher Tubbs wrote: