accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Keith Turner <ke...@deenlo.com>
Subject Re: [DISCUSS] MiniAccumuloCluster goals and approach
Date Wed, 26 Mar 2014 16:23:19 GMT
On Wed, Mar 26, 2014 at 12:12 PM, Josh Elser <josh.elser@gmail.com> wrote:

> On 3/26/14, 9:06 AM, Keith Turner 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.
>>
>
> That's my irk with it. The changes we made "hide" things for no other
> purpose than saying "we hid them". The next variant of a MAC is going to
> have to re-architect the entire thing anyways (I'm doing this right now and
> I'm overhauling it).
>

There is a purpose.  Whats an alternative solution to the addition of
"public List<LogWriter> getLogWriters()" to the MAC API?

If you want to re-write MAC all you have to support is the interface in
minicluster, you are free to throw everything in minicluster.impl away.


>
> It doesn't make sense to me ship something that changes it without
> addressing the underlying problems. I don't want to suggest it because I
> don't want to introduce our own mapred and mapreduce dichotomy, but I can't
> come up with a better alternative yet.
>
>
>  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.
>>
>
> Reorganizing of *this single implementation of MAC* can easily be done.
> Any extension or reimplementation will be dirty, and inherit a large bit of
> code that is likely useless.
>
>
>
>> 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