> 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
> 
>

Reply via email to