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: ACCUMULO-958 - Pluggable encryption in walogs
Date Wed, 30 Jan 2013 16:05:43 GMT
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 <afuchs@apache.org> 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 <eric.newton@gmail.com>
> 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 <josh.elser@gmail.com>
> 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
> anything.
> > I
> > > think for a security-minded database, we should probably be defaulting
> to
> > > fail-close; but, that brings up an issue, what happens when we can't
> > > encrypt a WAL? Do minor compactions fail gracefully? What does Accumulo
> > do?
> > >
> > > - John said he had been reviewing the patch before he applied it; it
> > > bothers me that there was a version of this patch that had been
> reviewed
> > > privately for some amount of time when we had already pushed back the
> > > feature freeze date by a week waiting for features that weren't done.
> > >
> > > - The author noted himself with the deprecation of the CryptoModule
> > > interface that "we anticipate changing [this] in non-backwards
> compatible
> > > ways as we explore requirements for encryption in Accumulo...". This
> > tells
> > > me that implementation of WAL encryption overall hasn't been properly
> > > thought out.
> > >
> > > Given all of this, it gives me great pause to knowingly include this
> > patch
> > > into a 1.5.0 release. I see no signs that this has been truly thought
> > out,
> > > there is no default provided encryption strategy for 1.5.0 with this
> > patch
> > > for the WAL and there is still no support at all for RFile encryption
> (no
> > > end-to-end Accumulo encryption for a user). All of these issues
> > considered
> > > make me believe that this is an incomplete feature that is not ready
> for
> > an
> > > Apache Accumulo release.
> > >
> > > Thoughts?
> > >
> > > - Josh
> > >
> > > [1] https://issues.apache.org/**jira/browse/ACCUMULO-958<
> > https://issues.apache.org/jira/browse/ACCUMULO-958>
> > >
> >
>

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