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).
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 <[email protected]>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
