commons-vfs2.jar 2.2 buggy

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

commons-vfs2.jar 2.2 buggy

Matthew Peterson
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
Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

Josh Elser-2
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
>
Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

Sean Busbey-7
sounds like a good DISCUSS thread for 2.0?
On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

Dave Marion
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 <[hidden email]>
wrote:

> sounds like a good DISCUSS thread for 2.0?
> On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email]> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

Christopher Tubbs-2
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 <[hidden email]> 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 <[hidden email]>
> wrote:
>
> > sounds like a good DISCUSS thread for 2.0?
> > On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email]> 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
> >
Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

Andrew Hulbert-2
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.

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 <[hidden email]> 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 <[hidden email]>
>> wrote:
>>
>>> sounds like a good DISCUSS thread for 2.0?
>>> On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email]> 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
>>>

Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

Andrew Hulbert-2
Forgot to mention...if I get the chance I'll try to peek at doDetach and
see if that's where we are running into the issue and maybe try to
contribute a better mechanism for jar files. But I guess it depends on
what Josh/Sean/Chris/etc think is best for 2.0.

On 10/26/2018 01:32 PM, Andrew Hulbert 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.
>
> 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 <[hidden email]> 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
>>> <[hidden email]>
>>> wrote:
>>>
>>>> sounds like a good DISCUSS thread for 2.0?
>>>> On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email]> 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
>>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

dlmarion
In reply to this post by Andrew Hulbert-2
Based on the comments in https://issues.apache.org/jira/browse/ACCUMULO-4828, the update to 2.2 did not solve the issues. Seems like reverting back to 2.1 might be in order for the short term.

> On October 26, 2018 at 1:32 PM Andrew Hulbert <[hidden email] mailto:[hidden email] > 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.
>
>     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 <[hidden email] mailto:[hidden email] > 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 <[hidden email] mailto:[hidden email] >
>     >> wrote:
>     >>
>     >>> sounds like a good DISCUSS thread for 2.0?
>     >>> On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email] mailto:[hidden email] > 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
>     >>>
>
Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

Matthew Peterson
Created https://github.com/apache/accumulo/pull/728 for the short term fix.

On Fri, Oct 26, 2018 at 1:50 PM Dave Marion <[hidden email]> wrote:

> Based on the comments in
> https://issues.apache.org/jira/browse/ACCUMULO-4828, the update to 2.2
> did not solve the issues. Seems like reverting back to 2.1 might be in
> order for the short term.
>
> > On October 26, 2018 at 1:32 PM Andrew Hulbert <[hidden email]
> mailto:[hidden email] > 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.
> >
> >     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 <
> [hidden email] mailto:[hidden email] > 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
> <[hidden email] mailto:[hidden email] >
> >     >> wrote:
> >     >>
> >     >>> sounds like a good DISCUSS thread for 2.0?
> >     >>> On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email]
> mailto:[hidden email] > 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
> >     >>>
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

Christopher Tubbs-2
In reply to this post by Andrew Hulbert-2
On Fri, Oct 26, 2018 at 1:34 PM Andrew Hulbert <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
> >> wrote:
> >>
> >>> sounds like a good DISCUSS thread for 2.0?
> >>> On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email]> 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
> >>>
>
Reply | Threaded
Open this post in threaded view
|

Re: commons-vfs2.jar 2.2 buggy

thomas.a.nelson@gmail.com


On 2018/10/30 13:41:24, Christopher <[hidden email]> wrote:

> On Fri, Oct 26, 2018 at 1:34 PM Andrew Hulbert <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
> > >> wrote:
> > >>
> > >>> sounds like a good DISCUSS thread for 2.0?
> > >>> On Wed, Oct 24, 2018 at 1:43 PM Josh Elser <[hidden email]> 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().