[
https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665497#action_12665497
]
Kay Kay commented on SOLR-972:
------------------------------
| interfaces are generally discouraged in Lucene/Solr because it can make the
API hard to modify. Though there is a slight inconvenience of creating a
separate class i guess it should be fine.
The rules about modification apply well enough for interfaces and abstract
classes. Having an abstract class limits the object hierarchy of the handler
unnecessary when it could be more flexible through the interfaces.
| We must find a better way to do this than making it a part of the object
lifecycle. We can add an extra scope to the Context#setSessionAttribute which
can persist between multiple invocations. This can hekp all the components to
share the same object
The persistence that I am concerned about is about the EventListener and less
about Context ( which is bound to be specific to be an event for most of the
time ). If the code in EventListener cannot store any state (fields ) - it is
not so intuitive to the developer of the plugin and not very efficient either.
| can we quantify that ?
Use of the reflection API ( Class.forName() , Class.newInstance() ) in any
piece of code that is going to be executed frequently ( delta import in this
case ) is hardly the best way to scale. I can come up with a sample unit test
case for that - but I think it is moot.
> EventListener-s creation changed from a per request ( full / delta-imports)
> scenario to once through the lifetime of the DIH plugin.
> ------------------------------------------------------------------------------------------------------------------------------------
>
> Key: SOLR-972
> URL: https://issues.apache.org/jira/browse/SOLR-972
> Project: Solr
> Issue Type: Improvement
> Components: contrib - DataImportHandler
> Environment: Java 6, Tomcat 6
> Reporter: Kay Kay
> Fix For: 1.4
>
> Attachments: SOLR-972.patch
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> The EventListener plugin for notification of start / end import events
> (SOLR-938) creates an instance of EventListener before every notification.
> This has 2 drawbacks.
> * No state is stored between successive invocations of events as it is a new
> object
> * When writing plugins for delta imports - it is very inefficient to do a
> class loader lookup by reflection / instantiate an instance and call a method
> on the same.
> Attached patch has one EventListener through the lifetime of the DIH plugin .
> Also EventListener is changed to an interface rather than an abstract class
> for better decoupling (especially painful when the start/end eventlistener
> has an independent hierarchy by itself ).
> By default, a no-op listener is registered to avoid boiler plate code to
> check if there is a start / end listener specified. Efficient JRE impls
> should be able to optimize the no-op for minimum overhead compared to
> checking the reference for null and branching out.
> Specifying an onImportStart / onImportEnd overrides the default handler
> though.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
