accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 24855: ACCUMULO-1454 design doc
Date Tue, 19 Aug 2014 19:26:20 GMT


> On Aug. 19, 2014, 6:21 p.m., Josh Elser wrote:
> > docs/src/main/asciidoc/design/ACCUMULO-1454-proposal-01.adoc, line 88
> > <https://reviews.apache.org/r/24855/diff/1/?file=664453#file664453line88>
> >
> >     If a user is programming to this API, how do they know what tservers are available?
Shouldn't there be a getTabletServers() method as well?
> >     
> >     Also, it would be better to return a concrete class instead of Iterable (since
we'd likely be backing it by some List). Advertise what we're actaully returning, and let
the user treat it as an Iterable if they so choose.
> 
> kturner wrote:
>     the following tablet server related methods already exist in instance operations.

>      
>      List<String> getTabletServers();
>      List<ActiveScan> getActiveScans(String tserver) throws AccumuloException,
AccumuloSecurityException;
>      List<ActiveCompaction> getActiveCompactions(String tserver) throws AccumuloException,
AccumuloSecurityException;
>      void ping(String tserver) throws AccumuloException;
>      
>      Using List would be consistent w/ rest of API.  I was think that Iterable would
allow it be backed by a scanner over the metadata table.   Was also thinking that something
like 
>      
>      ```loadTablets(getTablets("1.2.3.4:9997"), "1.2.3.4:9993")```
>      
>      does not have to load complete list of tablet into memory.  That may not be a worthwhile
goal.

API consistency would be best. As long as the results an API call to get TServers can be used
with the proposed new methods, I'm content.

Backing results with a Scanner can be nice, but dealing with concrete structures can also
be nice. Kind of have to guess the likelihood that the user is just going to wrap the Iterable
in a List/Set anyways (can probably punt on a decision until you actually write code which
uses the API).


> On Aug. 19, 2014, 6:21 p.m., Josh Elser wrote:
> > docs/src/main/asciidoc/design/ACCUMULO-1454-proposal-01.adoc, line 105
> > <https://reviews.apache.org/r/24855/diff/1/?file=664453#file664453line105>
> >
> >     If you're providing an unloadTablets method, I would think calling loadTablets
on a tablet that is already loaded should throw an Exception, not unload it for you.
> 
> kturner wrote:
>     My thinking was that load tablets would load on the specified tserver, irrespective
of the tablets current status.

That means that you would be in favor of an already loaded tablet on another tserver unloading
itself first (which means its essentially a move)? Or are you implying that the requested
load of an already assigned tablet would implicitly fail because that operation is impossible?


> On Aug. 19, 2014, 6:21 p.m., Josh Elser wrote:
> > docs/src/main/asciidoc/design/ACCUMULO-1454-proposal-01.adoc, line 111
> > <https://reviews.apache.org/r/24855/diff/1/?file=664453#file664453line111>
> >
> >     I'd lean towards keeping KeyExtent out of user's eyesight.
> 
> kturner wrote:
>     Thats what I am thinking.  I just checked where its used in API.  MutationsRejectedException,
ActiveScan, and ActiveCompaction use it.

The more I thought about it, as long as we clean it up, it's not the worst. The weird part
would be pushing the tablet identifier notation ('1;b;a') information into the "user's realm"
which makes me a little queasy. I'd rather have something more consumable for them.


> On Aug. 19, 2014, 6:21 p.m., Josh Elser wrote:
> > docs/src/main/asciidoc/design/ACCUMULO-1454-proposal-01.adoc, line 115
> > <https://reviews.apache.org/r/24855/diff/1/?file=664453#file664453line115>
> >
> >     These are going to be coming off of a connector or ZKI, right? I would treat
the instance id as implied (not required as an argument). host+port sounds good, but how do
you distinguish between localhost, 127.0.0.1, the FQDN and the external IP (if there aren't
many)?
> 
> kturner wrote:
>     I suppose the expectation w/ the current methods that take tserver as an argument
is that you will use something that came from getTabletServers().  I thik the string that
comes from that method is host+port, but not positive.

Could always standardize on the guava HostAndPort since that's what TServerUtils is doing
under the hoods anyways.


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24855/#review50998
-----------------------------------------------------------


On Aug. 19, 2014, 5:50 p.m., kturner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24855/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2014, 5:50 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-1454
>     https://issues.apache.org/jira/browse/ACCUMULO-1454
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Positing ACCUMULO-1454 design doc for review
> 
> 
> Diffs
> -----
> 
>   docs/src/main/asciidoc/design/ACCUMULO-1454-proposal-01.adoc PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24855/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kturner
> 
>


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