Thanks for your review! 2015-12-07 15:34 GMT+01:00 Alex Parvulescu <[email protected]>:
> thanks for the writeup, good stuff! > > I'd like to pick up some of the back references you mentioned for further > discussion: > > * org.apache.jackrabbit.oak.plugins.value.ValueImpl [0] : this looks like > something we can fix easily, just fix the Segment Property State impl to > catch potential SNFEs instead of the generic ValueImpl > > This is OAK-3740. I don't know if this approach works. PropertyState#getValue() doesn't declare checked exception, and RepositoryException is a checked exception. What about documenting in PropertyState#getValue() that implementers can throw an IllegalStateException to signal inconsistencies in the underlying code? In this case, ValueImpl could be implemented to always wrap IllegalStateExceptions in a RepositoryException. Moreover, SegemntNotFoundException already is a subclass of IllegalStateException. > * AbstractCheckpointMBean looks like a utility class to reduce code > duplication between both SegmentMk and DocumentMk. I'm actually wondering > if this reference is not an artifact of the refactoring and if we could > simply reference the abstract class name instead of the specific > SegmentCheckpointMBean class for the 'name' field [1]. > This is OAK-3741. I will try to parameterize the type of the MBean, adapting the different implementations when needed. Since I count only two subclasses of AbstractCheckpointMBean, this shouldn't be a lot of work. > > * FileStoreBackup & FileStoreRestore these are specific plugins which are > not big enough to have their own module, so options are: maintain a > 'plugins' package for SegmentMK bits in the new oak-core-segment bundle, or > move the plugins under the *segment package, which would defeat the point > of having plugin-like code in the first place. > This is OAK-3742. I will try to introduce a new interface to represent the construction of a NodeStore to use during the backup and restore operations. This way, I could be able to remove the direct dependency on the Segment Store from the implementation of those two classes. I don't have any further detail yet, I need to play a bit with the code and see how it goes. > > alex > > > [0] > > https://github.com/apache/jackrabbit-oak/blob/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueImpl.java#L379 > [1] > > https://github.com/apache/jackrabbit-oak/blob/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/util/AbstractCheckpointMBean.java#L62 > > > > On Mon, Dec 7, 2015 at 12:44 PM, Francesco Mari <[email protected]> > wrote: > > > Hi all, > > > > I started looking into moving the Segment Store and other classes closely > > related to its implementation to a new bundle, independent from > oak-core. I > > compiled a list of backwards and forward dependencies in [1]. > > > > By looking at this list, I immediately recognise two tasks. First, all > the > > backward dependencies should be removed. Second, it should be evaluated > if > > some packages should be exposed to allow the segment store to work as an > > independent bundle. > > > > Looking forward to your opinions. > > > > [1]: https://wiki.apache.org/jackrabbit/frm/SegmentStoreModularization > > >
