On 2018/10/30 13:41:24, Christopher <[email protected]> wrote:
> On Fri, Oct 26, 2018 at 1:34 PM Andrew Hulbert <[email protected]>
> wrote:
> >
> > Matt,
> >
> > We are running into similar issues with the 2.2 VFS jar running on
> > Accumulo 1.9.2 after upgrading from 1.8.1 but have been restarting
> > tservers to work around it and other issues with putting the iterators
> > in /tmp on certain systems.
> >
> > In general though we love it because we can run multiple versions of
> > iterators on the same cluster and we have it deployed on several systems
> > with our clients for that specific use case.
> >
> > Sean/Chris, if we rip it out would you imagine iterators being more like
> > HBase where you are basically bound to the startup classpath as the
> > baseline mechanism (with user-enabled specific class loaders). Or do you
> > imagine another upgrade/configuration mechanism? FYI we do VFS and the
> > general accumulo mechanism for configuring iterators and the iterator
> > api design because its pretty user/developer friendly.
> >
>
> My suggestion wasn't really about loss of functionality. Rather, the
> way I envision it, it is about supporting that same functionality with
> a modular design that is more maintainable, flexible, uses standard
> configuration mechanisms, and is properly segregated from the core
> code base.
>
> Even after "ripping out" our custom class loading from Accumulo's core
> code base, you could still do what you're doing today... it just might
> have to be configured more explicitly when you deploy your Accumulo
> cluster. And, the class loader code itself may be developed and
> maintained as a separate library from Accumulo itself. Accumulo
> wouldn't be tightly bound to that specific class loader
> implementation, and that specific implementation wouldn't be tightly
> bound to Accumulo.
>
> Regardless of changes to the startup class loading, we also have the
> per-table contexts in Accumulo, which allow separate class paths for
> each table, which we'd still support in some way (though perhaps in
> future, it could be implemented a bit better and configuring it would
> be more explicit).
>
> > Thanks,
> >
> > Andrew
> >
> >
> > On 10/24/2018 10:55 PM, Christopher wrote:
> > > The idea that Dave is talking about is that I don't think we should be
> > > doing any classloader special sauce in accumulo-start at all, and we
> > > might even be able to remove accumulo-start as a module entirely,
> > > since this is its primary (sole?) purpose.
> > >
> > > It's just a rough idea that I've tossed around with a few people, but
> > > haven't really spent any time materializing it into a proposal, PR, or
> > > experiment. Basically, I think we should rip out all classloader
> > > special sauce. If a user still wishes to use a custom classloader for
> > > any reason, using vfs2 or anything else, they can set a system class
> > > loader with -Djava.system.class.loader=my.custom.CustomClassLoader
> > > when they run java. This is an advanced Java option supported by Java
> > > itself, and really shouldn't be a problem to punt this downstream.
> > > Classloading is way outside the scope of what Accumulo does anyway,
> > > and Accumulo should have its complexity centered around what it does,
> > > and not "bells and whistles" on top of basic Java language functions.
> > >
> > > If we wanted to, we could use our current classloading code to create
> > > a classloader which could be used this way... and maybe provide it as
> > > an example or explain it in a blog post. But, Accumulo shouldn't be
> > > doing special sauce class loading... there are other projects that are
> > > better suited to specializing that for any Java application... and
> > > there's no reason we need it so tightly coupled to Accumulo.
> > >
> > > Of course, there's still some utility in the per-table context
> > > classloaders for pluggable components like iterators... and there's
> > > probably room for improvement in the configuration of those... but the
> > > main startup classloading is probably best to rip out.
> > >
> > > I'm not sure if it should be done for 2.0 or not... maybe yes. I'd be
> > > willing to rip it out... I enjoy ripping things out and reducing code
> > > complexity. But, I don't really have a desire to do the work of
> > > implementing or blogging about alternatives, if that's even necessary.
> > > I'd hope that somebody else would do that, if they felt it was really
> > > necessary once the built-in stuff was ripped out. For me, I'd be happy
> > > mentioning the feature in the release notes, maybe linking to the docs
> > > on the feature, and leaving implementation as an exercise for
> > > downstream, with an open invitation for a guest blog on our website
> > > about how it could be done.
> > >
> > > I've been thinking we're probably going to want a second alpha... or a
> > > beta, before 2.0 final... and if we did this for 2.0, I'd definitely
> > > want another pre-release release first.
> > >
> > > On Wed, Oct 24, 2018 at 3:10 PM Dave Marion <[email protected]> wrote:
> > >> I have talked with Christopher about the VFS class loader in general and
> > >> I
> > >> think he has a good approach. He can elaborate further if needed, but the
> > >> approach is to move it out of the core project and allow users to
> > >> configure
> > >> it at runtime using the java.system.class.loader system property. There
> > >> are
> > >> organizations using the VFSClassloader successfully, maybe it just needs
> > >> to
> > >> be reimplemented.
> > >>
> > >> On Wed, Oct 24, 2018 at 2:58 PM Sean Busbey <[email protected]>
> > >> wrote:
> > >>
> > >>> sounds like a good DISCUSS thread for 2.0?
> > >>> On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[email protected]> wrote:
> > >>>> It seems like commons-vfs2 is just a pile of crap.
> > >>>>
> > >>>> It's been known to have bugs for years and we've seen zero progress
> > >>>> from
> > >>>> them on making them better.
> > >>>>
> > >>>> IMO, rip the whole damn thing out.
> > >>>>
> > >>>> On 10/24/18 12:42 PM, Matthew Peterson wrote:
> > >>>>> Hello Accumulo,
> > >>>>>
> > >>>>> Summary: commons-vfs2 version 2.2 seems to have problems and it may be
> > >>>>> worth rolling back to version 2.1 of commons-vfs2.
> > >>>>>
> > >>>>> My project upgraded a system from Accumulo 1.8.1 to 1.9.2.
> > >>>>> Immediately
> > >>>>> after switching vfs contexts we saw problems. The tservers would
> > >>> error in
> > >>>>> iterators about missing classes that were clearly on the classpath.
> > >>> The
> > >>>>> problems were persistent until we replaced the commons-vfs2.jar with
> > >>>>> version 2.1 (Accumulo 1.9.2 uses version 2.2). Until we rolled vfs
> > >>> back,
> > >>>>> we received errors particularly with Spring code trying to access
> > >>> various
> > >>>>> classes and files within the jars. It looks like in 2.2, commons-vfs
> > >>>>> implemented a doDetach method which closed the zip files. We suspect
> > >>> that
> > >>>>> code is the problem but haven't tested that theory.
> > >>>>>
> > >>>>> I suspect that most users don't use this feature.
> > >>>>>
> > >>>>> Thanks!
> > >>>>> Matt
> > >>>>>
> > >>>
> > >>>
> > >>> --
> > >>> busbey
> > >>>
> >
> I wrote a unit test in commons-vfs2 that opens and accesses a jar file with
> multiple threads. It will often throw the same exception we see (that the
> ZipFileObject is closed). By changing the CacheStrategy to MANUAL instead of
> ON_RESOLVE the test no longer throws the exception because the new code in
> ZipFileObject::doDetach (which closes the file ZipFileSystem) is not called.
AbstractFileSystem::resolveFile checks if the CacheStrategy is ON_RESOLVE, and
if so, it calls file.refresh() which calls detach() which calls doDetach().
