Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Accumulo >> mail # dev >> ACCUMULO-958 - Pluggable encryption in walogs


Copy link to this message
-
Re: ACCUMULO-958 - Pluggable encryption in walogs
Bottom line, the patch has no value added to general users. The idea behind
pushing back a release date to stuff in unoperational code is very bad
practice. It sets a precedent for not considering alternative approaches
while simultaneously having no justification for choosing the approach we
did. If a specific customer/group/person wants a feature, and that feature
does not exist yet, the code is freely available to be modified,
distributed and open to public review. Adam, I strongly disagree that
forking the code is bad, considering the progress that other projects make
specifically because they have experimental forks (HBase).

On Wed, Jan 30, 2013 at 10:40 AM, Adam Fuchs <[EMAIL PROTECTED]> wrote:

> Let me attempt to make another argument for why the 958 patch should be
> included in 1.5.0. What this patch represents is not an encryption solution
> for WAL, but an experimental extension point that will be used for building
> an encryption solution as a pluggable module. We need to judge its merit
> based on whether it is a successful experimental extension point or not.
> There are three main reasons for including the patch in 1.5.0:
> 1. Test the performance impact of the null cipher solution (default
> configuration) in all the performance tests we will be running for the
> 1.5.0 release. If it causes problems there then we can roll it back.
> 2. Enable the use of this extension after 1.5 is released. External
> experiments have dependencies on this extension point. Without the
> extension point we will have to test with unreleased versions of Accumulo,
> which would be less than ideal.
> 3. It is not harmful and somebody wants it. The reason for wanting this
> code in is well documented, so you need a very strong reason to throw it
> out. Otherwise you will encourage forking of the project (which would be
> bad).
>
> Adam
>
>
>
>
> On Wed, Jan 30, 2013 at 10:09 AM, Eric Newton <[EMAIL PROTECTED]>
> wrote:
>
> > Some comments about the comments in ACCUMULO-958:
> >
> > Josh writes:
> >
> > "We still have the ability to review this even after the feature freeze
> > happens, it's just frustrating from my point of view in generating the
> best
> > 1.5.0 candidate possible (we tend to go through x.y.0 releases pretty
> darn
> > quick)."
> >
> > John writes:
> >
> > "Yes, but we get stuck on x.y.* for a year or so, so it does become a
> race
> > to get all the features you want to see in the next year."
> >
> > As Accumulo matures, we will need to start thinking a little more
> flexibly
> > about what goes into minor releases.  We have implemented new (small)
> > features in minor releases before.
> >
> > I would have no problem including ACCUMULO-958 into 1.5.1 after a test
> > phase, and after some basic experience with the feature.  However I'm
> very
> > uncomfortable including this in 1.5.0 because there is not a single test,
> > and no real implementation behind the factory that anyone would use In
> Real
> > Life.  Is this an appropriate API?  I have no idea.  Comments in the code
> > about the stability of the interface basically admit that the author
> isn't
> > completely comfortable with it, either.
> >
> > Let's not rush it, and when it is done right, I'm all for putting it into
> > the next release.  For now, I would hold back incorporating these changes
> > until they are more fully implemented. After we branch 1.5, commit this
> to
> > trunk, and back-port it to the 1.5 branch when experience and tests show
> it
> > is ready to be released.
> >
> > -Eric
> >
> >
> >
> > On Wed, Jan 30, 2013 at 9:13 AM, Josh Elser <[EMAIL PROTECTED]>
> wrote:
> >
> > > All,
> > >
> > > It's been a few days and I haven't seen much chatter at all on
> > > ACCUMULO-958 [1] since the patch was applied. There are a couple of
> > > concerns I have that I definitely want to see addressed before a 1.5.0
> > > release.
> > >
> > > - It worries me that the provided patch is fail-open (when we can't
> load
> > > the configured encryption strategies/modules, we don't decrypt