You are viewing a plain text version of this content. The canonical link for it is here.
Posted to solr-dev@lucene.apache.org by "Kay Kay (JIRA)" <ji...@apache.org> on 2009/01/19 23:18:59 UTC
[jira] Created: (SOLR-972) EventListener are not created per
request ( full / delta-imports) but rather instantiated once per lifetime
of the application - maintaining state + efficient
EventListener are not created per request ( full / delta-imports) but rather instantiated once per lifetime of the application - maintaining state + efficient
---------------------------------------------------------------------------------------------------------------------------------------------------------------
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
Attachments: SOLR-972.patch
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.
[jira] Updated: (SOLR-972) EventListener are not created per
request ( full / delta-imports) but rather instantiated once per lifetime
of the application - maintaining state + efficient
Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kay Kay updated SOLR-972:
-------------------------
Attachment: SOLR-972.patch
> EventListener are not created per request ( full / delta-imports) but rather instantiated once per lifetime of the application - maintaining state + efficient
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> 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.
[jira] Updated: (SOLR-972) EventListener-s creation changed from a
per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kay Kay updated SOLR-972:
-------------------------
Summary: EventListener-s creation changed from a per request ( full / delta-imports) scenario to once through the lifetime of the DIH plugin. (was: EventListener are not created per request ( full / delta-imports) but rather instantiated once per lifetime of the application - maintaining state + efficient )
> 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.
[jira] Issue Comment Edited: (SOLR-972) EventListener-s creation
changed from a per request ( full / delta-imports) scenario to once through
the lifetime of the DIH plugin.
Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666042#action_12666042 ]
noble.paul edited comment on SOLR-972 at 1/21/09 8:04 PM:
----------------------------------------------------------
bq.I am not sure if we can apply such generality since Context is bound to be different for different invocations
The Context itself will not be same for different invocations but the behavior of the new one is going to be consistent
imagine we have a scope "dataimporter" and you can set an attribute at that scope by calling
{code}
context.setAttribute("any_name",the_value,"dataimporter");
//some time later you can get back the same object
Object the_value = context.getAttribute("any_name","dataimporter");
//do my ops here
{code}
It is still possible for you to share Objects in a static variable in your EventListener.
was (Author: noble.paul):
bq.I am not sure if we can apply such generality since Context is bound to be different for different invocations
The Context itself will not be same for different invocations but the behavior of the new one is going to be consistent
imagine we have a scope "dih" and you can set an attribute at that scope by calling
{code}
context.setAttribute("any_name",the_value,"dih");
//some time later you can get back the same object
Object the_value = context.getAttribute("any_name""dih");
//do my ops here
{code}
It is still possible for you to share Objects in a static variable in your EventListener.
> 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12667580#action_12667580 ]
Noble Paul commented on SOLR-972:
---------------------------------
hi Kay, I am -1 on giving special treatment to an EventListener. The behavior has to be consistent w/ that of other components.
> 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666042#action_12666042 ]
Noble Paul commented on SOLR-972:
---------------------------------
bq.I am not sure if we can apply such generality since Context is bound to be different for different invocations
The Context itself will not be same for different invocations but the behavior of the new one is going to be consistent
imagine we have a scope "dih" and you can set an attribute at that scope by calling
{code}
context.setAttribute("any_name",the_value,"dih");
//some time later you can get back the same object
Object the_value = context.getAttribute("any_name""dih");
//do my ops here
{code}
It is still possible for you to share Objects in a static variable in your EventListener.
> 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12667643#action_12667643 ]
Shalin Shekhar Mangar commented on SOLR-972:
--------------------------------------------
bq. 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.
Not really, since you don't necessarily need to keep the new methods abstract, they can be a no-op. Thus you maintain binary compatibility with old code. One doesn't have this luxury with interfaces.
bq. Imagine a requesthandler in the Servlet having no state and being instantiated every time for every request. By recreating EventListener-s , we have a similar analogy. That would make (it already does and I am working on patched version of Solr) development harder to share any state between successive calls.
I don't think that is an apples to apples comparison. Servlets are supposed to be called a lot many times per second. Also, containers may create many instances of servlets.
bq. 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.
I try to stay away from micro-optimizations (which is what I felt this issue was about) as they don't add any value to the system. One wouldn't even be able to see any measurable performance difference unless one calls delta-imports hundreds of times per second. Even if someone did, the bottleneck would probably be the database rather than creation of event listeners.
If this issue is more about maintaining state rather than the optimization, can you see if SOLR-988 can solve your problem? Most objects in DIH are created per import request and I'd like to keep it that way. I agree with Noble that EventListener cannot be seen as an exception. There are enough inconsistencies already and I'd like to cut them down rather than add one more exception.
> 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665702#action_12665702 ]
Noble Paul commented on SOLR-972:
---------------------------------
bq.The persistence that I am concerned about is about the EventListener and less about Context
The context is the single interaction point for all components with the system. So it is better to be there. DIH does not yet guarantee any state persistence between two imports . If it is a part of the Context API and documented it becomes a standard way of storing data for all components. EventListener cannot be seen as a special component.
All components are created on a per import basis. If we must cache , then we must think of it for or all the the components
> 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.
[jira] Updated: (SOLR-972) EventListener are not created per
request ( full / delta-imports) but rather instantiated once per lifetime
of the application - maintaining state + efficient
Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kay Kay updated SOLR-972:
-------------------------
Fix Version/s: 1.4
> EventListener are not created per request ( full / delta-imports) but rather instantiated once per lifetime of the application - maintaining state + efficient
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12667287#action_12667287 ]
Kay Kay commented on SOLR-972:
------------------------------
Any idea when this patch is going to be committed.
> 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.
[jira] Issue Comment Edited: (SOLR-972) EventListener-s creation
changed from a per request ( full / delta-imports) scenario to once through
the lifetime of the DIH plugin.
Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666042#action_12666042 ]
noble.paul edited comment on SOLR-972 at 1/21/09 8:09 PM:
----------------------------------------------------------
bq.I am not sure if we can apply such generality since Context is bound to be different for different invocations
The Context itself will not be same for different invocations but the behavior of the new one is going to be consistent
imagine we have a scope "dataimporter" and you can set an attribute at that scope by calling
{code}
context.setAttribute("any_name",the_value,"dataimporter");
//some time later you can get back the same object
Object the_value = context.getAttribute("any_name","dataimporter");
//do my ops here
{code}
It is still possible for you to share Objects in a static variable in your EventListener.
The design is modeled like the servlet API. This is akin to storing and retrieving data from the servletContext,session,request etc .
was (Author: noble.paul):
bq.I am not sure if we can apply such generality since Context is bound to be different for different invocations
The Context itself will not be same for different invocations but the behavior of the new one is going to be consistent
imagine we have a scope "dataimporter" and you can set an attribute at that scope by calling
{code}
context.setAttribute("any_name",the_value,"dataimporter");
//some time later you can get back the same object
Object the_value = context.getAttribute("any_name","dataimporter");
//do my ops here
{code}
It is still possible for you to share Objects in a static variable in your EventListener.
> 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.
[jira] Resolved: (SOLR-972) EventListener-s creation changed from a
per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Shalin Shekhar Mangar resolved SOLR-972.
----------------------------------------
Resolution: Won't Fix
An alternative exists for maintaining state and EventListeners should not be treated in a special manner. Therefore I'm marking this as "Won't Fix".
> 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666193#action_12666193 ]
Kay Kay commented on SOLR-972:
------------------------------
I agree with the comment regarding a custom scope for a Context . For those Context-s that need to be reused this could still be in scope before fetching to avoid recreating them again but I am more concerned about recreating EventListener-s .
| It is still possible for you to share Objects in a static variable in your EventListener.
| The design is modeled like the servlet API. This is akin to storing and retrieving data from the servletContext,session,request etc .
Sharing via static variables does not seem to be the cleanest way to design . (What if there are 2 eventListeners one for start and another for end inheriting from a common class that has shared attributes. Sharing via static variables ( in the base class) brings unpredictable behavior / and a code difficult to maintain . )
Imagine a requesthandler in the Servlet having no state and being instantiated every time for every request. By recreating EventListener-s , we have a similar analogy . That would make (it already does and I am working on patched version of Solr) development harder to share any state between successive calls.
> 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Noble Paul (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665328#action_12665328 ]
Noble Paul commented on SOLR-972:
---------------------------------
bq.Also EventListener is changed to an interface rather than an abstract class for better decoupling
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.
bq.No state is stored between successive invocations of events as it is a new object
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
bq.- it is very inefficient to do a class loader lookup by reflection / instantiate an instance and call a method on the same.
can we quantify that ?
> 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Commented: (SOLR-972) EventListener-s creation changed from
a per request ( full / delta-imports) scenario to once through the lifetime
of the DIH plugin.
Posted by "Kay Kay (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665842#action_12665842 ]
Kay Kay commented on SOLR-972:
------------------------------
| All components are created on a per import basis. If we must cache , then we must think of it for or all the the components
I am not sure if we can apply such generality since Context is bound to be different for different invocations (at least , most of the time) whereas EventListener usually take the form of register / unregister but are not created every time. If EventListener-s are going to be recreated everytime then it would mean that the impl. has no state ( fields ) in it - which seems not so intuitive to the programmer.
> 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.