You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Chris Hostetter <ho...@fucit.org> on 2014/09/04 00:40:25 UTC

Re: Solr Plugins Class Loader Issue / Custom DistributedUpdateProcessor

: After a lot of debugging I found out that you cannot extend Solr/Lucene
: classes and override package protected methods, it will silently us the
: super class' method.
	...
: Has anyone happened to come across this and know if there is a fix for
: extending & overriding package protected methods?

That's a rule of Java - not anything special about Solr/Lucene.

package protected means it's only visible to classes in the same package 
-- jus because you (a human) can read that method, doesn't mean your code 
has any knowledge of it's existence -- you can't override it in a 
subclass, nor can you call it.  The compiler will fail hard if you try the 
later, and warn you of the former if you use the "@Override" annotation 
(it can't warn you of anything w/o the annotation, because it can't even 
see the method to know that there is anything to warn you of - you have to 
explicitly say you are epxecting to override something for it to be able 
to tell you "Hey, wait a minute - no you aren't")

"package protected" is generally used either as an alternative to 
"public" (when we only want other solr code in the same pacakge to access 
it) or as an alterantive to "private" (in cases where ideally no one 
outside the class should call that method, but we still want to be able to 
unit test it).  In either case the objective is to minimize the surface 
area of the API and hide implementation details.

: For a little context, I came across this issue because a client requires
: some slightly modified merge logic within the DistributedUpdateProcessor.

Off the top of my head, i'm not sure what the motivation was here in this 
specific case, or if/why RequestReplicationTracker shouldn't be public -- 
but my suggestion would be to open a jira proposing the specific access
changes you think make sense (to the method in question, and/or 
the new inner class) to make the code more re-usable and see what folks 
think about exposing that bit of the API as "supported"

First though, you may want to take a step back, and instead focus on a 
discussion/jira of what the "slightly modified merge logic" is that you 
are currently using, and propose new hooks for you to do that in the most 
optimal way (not sure if the answer's are the same ... don't want to 
assume this isn't the birth of an XY problem)



-Hoss
http://www.lucidworks.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Re: Solr Plugins Class Loader Issue / Custom DistributedUpdateProcessor

Posted by Steve Davids <sd...@gmail.com>.
Ah, that explains it - thanks Hoss! I wasn’t aware of this class loader subtlety, thanks for teaching me something new today :)

-Steve

On Sep 3, 2014, at 7:53 PM, Chris Hostetter <ho...@fucit.org> wrote:

> 
> : Yes, I specifically made a package in my custom plugins code that was in 
> : the solr package space i.e.: org.apache.solr.update.processor
> 	...
> : completely fine within my unit tests as it was loaded with the same 
> : class loader (junit test run). I believe the specific issue that I came 
> 	...
> : something is a bit screwy with the external resource loader as this 
> : should work (as long as the custom class is within the same package name 
> : as Solr).
> 
> Ah, ok - so yeah: what you were doing at compile time was valid, but by 
> then loading those "org.apache.solr.*" classes as a plugin (vs putting 
> them in the war with the other classes in that pacakge) is definitely what 
> caused the problem -- at runtime class/package identity includes the 
> ClassLoader...
> 
> http://www.cooljeff.co.uk/2009/05/03/the-subtleties-of-overriding-package-private-methods/
> 
> ie: not a bug in Solr's plugin ClassLoader, just the devil-in-the-details 
> of how a "package" is defined as far as the runtime goes.  (And 
> unfortunately, the "@Override" check in java is source annotation only - 
> there's no way to ask the JVM to enforce that and treat it as an assertion 
> or anything like that)
> 
> 
> 
> -Hoss
> http://www.lucidworks.com/
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Re: Solr Plugins Class Loader Issue / Custom DistributedUpdateProcessor

Posted by Chris Hostetter <ho...@fucit.org>.
: Yes, I specifically made a package in my custom plugins code that was in 
: the solr package space i.e.: org.apache.solr.update.processor
	...
: completely fine within my unit tests as it was loaded with the same 
: class loader (junit test run). I believe the specific issue that I came 
	...
: something is a bit screwy with the external resource loader as this 
: should work (as long as the custom class is within the same package name 
: as Solr).

Ah, ok - so yeah: what you were doing at compile time was valid, but by 
then loading those "org.apache.solr.*" classes as a plugin (vs putting 
them in the war with the other classes in that pacakge) is definitely what 
caused the problem -- at runtime class/package identity includes the 
ClassLoader...

http://www.cooljeff.co.uk/2009/05/03/the-subtleties-of-overriding-package-private-methods/

ie: not a bug in Solr's plugin ClassLoader, just the devil-in-the-details 
of how a "package" is defined as far as the runtime goes.  (And 
unfortunately, the "@Override" check in java is source annotation only - 
there's no way to ask the JVM to enforce that and treat it as an assertion 
or anything like that)



-Hoss
http://www.lucidworks.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Re: Solr Plugins Class Loader Issue / Custom DistributedUpdateProcessor

Posted by Steve Davids <sd...@gmail.com>.
: That's a rule of Java - not anything special about Solr/Lucene.

Yes, I specifically made a package in my custom plugins code that was in the solr package space i.e.: org.apache.solr.update.processor

I did add the @Override to that specific package protected class and it did properly compile with no warnings or errors. This also worked completely fine within my unit tests as it was loaded with the same class loader (junit test run). I believe the specific issue that I came across was that the Solr WAR is loaded by the web container’s class loader while the separate “plugins” jar is loaded by Solr’s custom class loader. I believe at that point since the two were loaded separately the custom processor didn’t properly override the package protected method, though I do know the constructor was at least being called on the custom processor via various debugging/logging.

I will open a ticket about custom merge logic soon, though I do believe something is a bit screwy with the external resource loader as this should work (as long as the custom class is within the same package name as Solr).

-Steve

On Sep 3, 2014, at 6:40 PM, Chris Hostetter <ho...@fucit.org> wrote:

> 
> : After a lot of debugging I found out that you cannot extend Solr/Lucene
> : classes and override package protected methods, it will silently us the
> : super class' method.
> 	...
> : Has anyone happened to come across this and know if there is a fix for
> : extending & overriding package protected methods?
> 
> That's a rule of Java - not anything special about Solr/Lucene.
> 
> package protected means it's only visible to classes in the same package 
> -- jus because you (a human) can read that method, doesn't mean your code 
> has any knowledge of it's existence -- you can't override it in a 
> subclass, nor can you call it.  The compiler will fail hard if you try the 
> later, and warn you of the former if you use the "@Override" annotation 
> (it can't warn you of anything w/o the annotation, because it can't even 
> see the method to know that there is anything to warn you of - you have to 
> explicitly say you are epxecting to override something for it to be able 
> to tell you "Hey, wait a minute - no you aren't")
> 
> "package protected" is generally used either as an alternative to 
> "public" (when we only want other solr code in the same pacakge to access 
> it) or as an alterantive to "private" (in cases where ideally no one 
> outside the class should call that method, but we still want to be able to 
> unit test it).  In either case the objective is to minimize the surface 
> area of the API and hide implementation details.
> 
> : For a little context, I came across this issue because a client requires
> : some slightly modified merge logic within the DistributedUpdateProcessor.
> 
> Off the top of my head, i'm not sure what the motivation was here in this 
> specific case, or if/why RequestReplicationTracker shouldn't be public -- 
> but my suggestion would be to open a jira proposing the specific access
> changes you think make sense (to the method in question, and/or 
> the new inner class) to make the code more re-usable and see what folks 
> think about exposing that bit of the API as "supported"
> 
> First though, you may want to take a step back, and instead focus on a 
> discussion/jira of what the "slightly modified merge logic" is that you 
> are currently using, and propose new hooks for you to do that in the most 
> optimal way (not sure if the answer's are the same ... don't want to 
> assume this isn't the birth of an XY problem)
> 
> 
> 
> -Hoss
> http://www.lucidworks.com/
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>