accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From William Slacum <wilhelm.von.cl...@accumulo.net>
Subject Re: [DISCUSS] MiniAccumuloCluster goals and approach
Date Wed, 26 Mar 2014 17:42:30 GMT
Correction from my previous email:

At this point, the MiniAccumuloCluster's interface of the
MiniAccumuloClusterImpl's interface.

should read

At this point, the MiniAccumuloCluster's interface is a subset of the
MiniAccumuloClusterImpl's interface.


On Wed, Mar 26, 2014 at 1:10 PM, William Slacum <
wilhelm.von.cloud@accumulo.net> wrote:

> [NOTE: I started this email when this thread was new, and it kinda of blew
> up on me while writing it and being distracted. Apologies in advance if
> things were already covered or it's not relevant any more.]
>
> Is this a design quality discussion or a a functionality discussion?
>
> The changes from 1.5->1.6 seem like a poor design decision, but they do
> aid in functionality.
>
> From 1.5:
>   public MiniAccumuloCluster(File dir, String rootPassword) throws
> IOException
>   public MiniAccumuloCluster(MiniAccumuloConfig config) throws IOException
>   public void start() throws IOException, InterruptedException
>   public String getInstanceName()
>   public String getZooKeepers()
>   public void stop() throws IOException, InterruptedException
>
> From 1.6:
>   public MiniAccumuloCluster(File dir, String rootPassword) throws
> IOException
>   public MiniAccumuloCluster(MiniAccumuloConfig config) throws IOException
>   public void start() throws IOException, InterruptedException
>   public Set<Pair<ServerType,Integer>> getDebugPorts()
>   public String getInstanceName()
>   public String getZooKeepers()
>   public void stop() throws IOException, InterruptedException
>   public MiniAccumuloConfig getConfig()
>   public Connector getConnector(String user, String passwd) throws
> AccumuloException, AccumuloSecurityException
>   public ClientConfiguration getClientConfig()
>
> From a client perspective, I see a difference of #getDebugPorts,
> #getConfig, #getConnector, #getClientConfig. The other methods are on the
> Impl. There's nothing wrong with using aggregation in this case, since the
> code would be the same regardless.
>
> I don't quite understand what it means to "extend generically." At this
> point, the MiniAccumuloCluster's interface of the MiniAccumuloClusterImpl's
> interface. The naming could, and should, be better, but I don't quite get
> where we're losing functionality.
>
>
>
> On Wed, Mar 26, 2014 at 12:06 PM, Keith Turner <keith@deenlo.com> wrote:
>
>> There were many change made to MAC so Accumulo could test itself.  For
>> example a method was added to return the internal threads that flush logs.
>> Flushing the logs may be a useful feature to add.  However it could be
>> offered in a way that does not expose these internal threads.   When
>> working on  ACCUMULO-2151 I had no desire to reimplement things like this,
>> I just wanted to hide it.  It was hidden from users so we do not have to
>> support it and can change it at will when testing 1.7.0.
>>
>> As Sean said MAC was a class in 1.4.4, 1.5.0, and 1.5.1.  So making it an
>> interface would break things for any users using it.  Any reorganizing of
>> the implementation of MAC could easily be done after 1.6.0.  From a users
>> perspective the MAC API has changed very little, even though the
>> implementation has dramatically changed.
>>
>>
>>
>>
>> On Wed, Mar 26, 2014 at 3:10 AM, Sean Busbey <busbey+lists@cloudera.com
>> >wrote:
>>
>> > ACCUMULO-2143 has developed a conversation about MiniAccumuloCluster's
>> > intended use and the way we currently implement the difference between
>> MAC
>> > for external use and MAC for internal Accumulo testing[1].
>> >
>> > In particular, Josh had a few major concerns
>> >
>> > -----
>> >
>> > It doesn't make sense to me why MiniAccumuloCluster is a concrete class
>> > which, ultimately still tied to a MiniAccumuloClusterImpl.
>> > MiniAccumuloCluster *requires* a MiniAccumuloClusterImpl or something
>> that
>> > extends it. This is what's really chafing me about the separation of
>> > "accumulo user" and "accumulo developer" methods - you *always* have
>> them
>> > both. Not to mention, this hierarchy is really obnoxious to create a new
>> > implementation of AccumuloMiniCluster(Impl) because I have to carry all
>> of
>> > the cruft of the "original" implementation with me.
>> >
>> > Bringing this back around to this ticket, while I still don't agree with
>> > the reasoning that exposing the FileSystem or ZooKeeper object that
>> > MiniAccumuloClusterImpl is getting us anything other than the ability to
>> > say "we didn't change this [arbitrary] API". For "users" who might not
>> care
>> > what the underlying FileSystem or ZooKeeper connection, it's merely an
>> > extra two items in their editor's code-completion. For "users" who would
>> > care to use this information, we now make them jump through extra hoops
>> to
>> > get it. That just doesn't make any sense to me for something we haven't
>> > even released.
>> >
>> > To be honest, I really want to re-open
>> > ACCUMULO-2151<https://issues.apache.org/jira/browse/ACCUMULO-2151>,
>> > make MiniAccumuloCluster an interface, MiniAccumuloClusterImpl an
>> > implementation of said interface, and create some factory class to make
>> > instances, ala Connector.tableOperations, Connector.securityOperations,
>> > etc. Right now there's a class we call an "API" that cannot be
>> generically
>> > extended for the sake of saying "we have an API".
>> >
>> > ----
>> >
>> > I wanted to avoid having a drawn out discussion on a jira, where folks
>> my
>> > not notice it. Especially with things being late in 1.6.0 development
>> and
>> > the potential this has to impact the API.
>> >
>> > Personally, I don't have much of a dog in the fight. There's always some
>> > arbitrary line for where the public API will be, presuming we want to
>> have
>> > any kind of balance between providing a stable based for others to
>> build on
>> > and being able to refactor things. I would like us to hold to our API
>> > promises[2] and I would rather we not leak implementation details
>> > unnecessarily.
>> >
>> > I suspect the choice to make MiniAccumuloCluster a class rather than an
>> > interface with a factory was driven by the restrictions we put on API
>> > changes between major versions and the fact that 1.5 had a class you
>> could
>> > instantiate via constructors[3].
>> >
>> > It's possible we can address some of the major reusability concerns by
>> > moving most of the implementation back into MAC, liberally using package
>> > access for members, and making the internal-use MAC extend the public
>> one.
>> >
>> >
>> > [1]: https://issues.apache.org/jira/browse/ACCUMULO-2143
>> > [2]: http://accumulo.apache.org/governance/releasing.html
>> > [3]: https://issues.apache.org/jira/browse/ACCUMULO-2151
>> >
>>
>
>

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