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 > 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> > > > > > >
