> On March 18, 2014, 7:39 p.m., Sean Busbey wrote: > > which branch is this against? > > Mike Drob wrote: > Probably should be against 1.5.1, but I'm not sure. > > Sean Busbey wrote: > I'd like to see this against all active dev branches, starting with > 1.4.5-SNAP. I can file a follow on backport ticket if you like. > > kturner wrote: > in 1.6.0-SNAPSHOT, the function to get the std dev seems to only be used > in test code. If this is the case in earlier relases, then thats not a very > strong case for applying it. > > Josh Elser wrote: > Please, let's avoid backports for new changes that come in. Put in the > correct place the first time. > > Sean Busbey wrote: > yeah, I'd prefer that this start in 1.4.5 and then get brought forward to > master. > > Keith, I get the desire to avoid maintenance but I think having known > incorrect behavior with a compatible fix in versions we haven't EOLed yet is > going to cause more problems going forward. Doubly so if the error is in > something our tests are built upon. > > kturner wrote: > Not trying to avoid maintenance, just thinking about risk and benefits. > I have seen multiple times in the past where small seemingly innocuous > changes for minor bugs have introduced critical bugs. In this case > TabletServer uses the Stat class, but does not use the std deviation. The > risk is a small possibility of introducing a new critical bug in tserver if > the change breaks Stat in some strange new way. The benefit of the change is > that informational output from a few test may be better.
The only thing I can think of, is if there is some strange performance-related issue that comes out of switching to using commons-math. Since 1.4 does not yet depend on commons-math, I don't want to introduce that. However, since 1.5 does, we could fix retrofit the stdev function to use it there, and then replace the full implementation in 1.6 and newer? or swap out the full implementation in just master? Thoughts on this proposed compromise? - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19368/#review37610 ----------------------------------------------------------- On March 18, 2014, 8:40 p.m., Mike Drob wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19368/ > ----------------------------------------------------------- > > (Updated March 18, 2014, 8:40 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2494 > https://issues.apache.org/jira/browse/ACCUMULO-2494 > > > Repository: accumulo > > > Description > ------- > > ACCUMULO-2494 Delegate math to commons-math > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/util/Stat.java > e65265c6decde47ef229377653112a677fef8112 > core/src/test/java/org/apache/accumulo/core/util/StatTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/19368/diff/ > > > Testing > ------- > > Unit tests results compared with calculations from Wolfram Alpha. > > > Thanks, > > Mike Drob > >
