> On Dec. 10, 2014, 2:54 a.m., Christopher Tubbs wrote: > > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java, > > lines 581-586 > > <https://reviews.apache.org/r/28873/diff/1/?file=787893#file787893line581> > > > > +1 for this check, but this should be a separate issue, and also would > > benefit 1.6 as a sanity-check/bug-prevention measure. > > Sean Busbey wrote: > It's not strictly needed in 1.6 because only someone with control of > accumulo-site.xml can set the volume chooser, but I'm happy to file a follow > on. > > Christopher Tubbs wrote: > We also allow overriding general system properties in ZooKeeper, so I'm > not sure that's exactly true. I'd have to check which properties we permit to > be changed that way. In any case, to use a particular VolumeChooser, we > already require it to be on the classpath, which requires the same level of > privilege as modifying the accumulo-site.xml file, so I'm not sure this is > distinctly different between 1.6 and master.
I went ahead and created ACCUMULO-3400 for follow-on for 1.6. I couldn't figure out how to word it so it was scoped for 1.6, but not for 1.7, so I just made it to encompass the issue across versions. I think it'd be good to just go right ahead and implement this change under that issue (I don't think there's any contention at all over that check), so we can focus this issue on the remaining clean-up/changes expressed in the remainder of this patch. - Christopher ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28873/#review64511 ----------------------------------------------------------- On Dec. 10, 2014, 9:58 a.m., Sean Busbey wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28873/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2014, 9:58 a.m.) > > > Review request for accumulo and Jenna Huston. > > > Bugs: ACCUMULO-3393 > https://issues.apache.org/jira/browse/ACCUMULO-3393 > > > Repository: accumulo > > > Description > ------- > > ACCUMULO-3393 Follow-on work for per-table volume chooser. > > Still has TODOs for additional tickets I need to file. I'll update the review > to remove once I have them all filed. I think everything marked TODO can wait > for a later ticket. Please comment on relevant section if you think something > needs to be done now. > > * docs clean up + code guideline compliance. > * ensure RandomVolumeChoosers are independent when used per-table. > * make sure that per-table choosers can keep state the way that global > choosers can > * make sure that a chooser can only pick from the options it is presented. > * avoid object creation in critical path for Tablet.split. > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/conf/Property.java > 4c2d0b41b3bce32449861da3ac42fa27deb2b182 > pom.xml 31601a1bf84e19b861e4f48b50824eeb77987b52 > server/base/pom.xml c21a168dec2092692f1ee6877c4703ee2d3e3977 > > server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java > 7a825c796eb5a25de8c748e2aba642f483697b9a > > server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java > 7ed7bba809a4e5e4b2d472c3327b15adb37251a7 > > server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java > f2eb2113cb848ed58ac5f41573c6ff2cde9b0a77 > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java > f523057b11a2dc42e82010675bb1ac8e3802f96d > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java > 890651e92f4c34514cb839b7b9ee9d23ad55070a > > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java > dc1be739b634d91992894f8f27c2d9c184bd98cd > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java > 877d01c2233cca086c1ac1539eb81cc282a7ceb4 > > server/base/src/main/java/org/apache/accumulo/server/util/TabletOperations.java > c0e1a9b991d61a4dbb127c74ae297f171434e7d5 > > server/base/src/test/java/org/apache/accumulo/server/fs/VolumeManagerImplTest.java > 582822a8ccbc398925a2184eed0b9d7fa853f9b4 > > Diff: https://reviews.apache.org/r/28873/diff/ > > > Testing > ------- > > Ran through VolumeManagerImplTest and VolumeChooserIT > > > Thanks, > > Sean Busbey > >
