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 "Grant Ingersoll (JIRA)" <ji...@apache.org> on 2007/11/28 20:24:43 UTC

[jira] Created: (SOLR-423) SolrRequestHandler close notification

SolrRequestHandler close notification
-------------------------------------

                 Key: SOLR-423
                 URL: https://issues.apache.org/jira/browse/SOLR-423
             Project: Solr
          Issue Type: Improvement
            Reporter: Grant Ingersoll
            Assignee: Grant Ingersoll
            Priority: Minor


It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SOLR-423) SolrRequestHandler close notification

Posted by "Grant Ingersoll (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Grant Ingersoll resolved SOLR-423.
----------------------------------

    Resolution: Fixed

Committed revision 671960.

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>    Affects Versions: 1.3
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>             Fix For: 1.3
>
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-423) SolrRequestHandler close notification

Posted by "Grant Ingersoll (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Grant Ingersoll updated SOLR-423:
---------------------------------

    Attachment: SOLR-423.patch

Draft patch that implements close() callback on the SolrRequestHandler and provides empty implementation for RequestHandlerBase.

Obviously, this is not backward compatible since it adds a new method to an interface, but I am not sure where else to put it for now.  My guess is most people extend RequestHandlerBase anyway, so it may not be that big of an issue.

Patch hooks into the SolrCore.close() call by calling RequestHandlers.closeHandlers() similar to how it inits the handlers.

All tests should pass.  Also added a test to SolrCoreTest to explicitly check that close is being called.

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-423) SolrRequestHandler close notification

Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12548513 ] 

Hoss Man commented on SOLR-423:
-------------------------------

> How would this work? I don't quite follow. If we add close() to 
> SolrEventListener that would break the API.

...sigh....   you're 100% right.  It's been a while since i looked at that API, i forgot it was an interface (heck: i forgot the event types were different methods)  oh well.

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-423) SolrRequestHandler close notification

Posted by "Ryan McKinley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12546851 ] 

Ryan McKinley commented on SOLR-423:
------------------------------------

> That then got me thinking that instead of a CloseHook we should probably have a generic "closeCore" event type ... users could configure explicit SolrEventListeners in the solrconfig.xml, and plugins could use the internal methods to add implicit event listeners that they need.

How would this work?  I don't quite follow.  If we add close() to SolrEventListener that would break the API.

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-423) SolrRequestHandler close notification

Posted by "Grant Ingersoll (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Grant Ingersoll updated SOLR-423:
---------------------------------

    Attachment: SOLR-423.patch

Minor updates, added some javadocs, plan to commit soon.

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>    Affects Versions: 1.3
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>             Fix For: 1.3
>
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-423) SolrRequestHandler close notification

Posted by "Ryan McKinley (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Ryan McKinley updated SOLR-423:
-------------------------------

    Attachment: SOLR-423-CloseHook.patch

This version adds a CloseHook to SolrCore.  The test now looks like:

{code:java}
class ClosingRequestHandler extends EmptyRequestHandler implements SolrCoreAware {
  boolean closed = false;

  public void inform(SolrCore core) {
    core.addCloseHook( new SolrCore.CloseHook() {
      public void close(SolrCore core) {
        closed = true;
      }
    });
  }
}
{code}

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-423) SolrRequestHandler close notification

Posted by "Hoss Man (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12546434 ] 

Hoss Man commented on SOLR-423:
-------------------------------

i was thinking the same thing about #1 but that got me thinking that instead of SolrCoreAware having a close method, SolrCore could have an "addCloseHook(CloseHook) method which SolrCoreAware request handlers could call as part their inform() method.

That then got me thinking that instead of a CloseHook we should probably have a generic "closeCore" event type ... users could configure explicit SolrEventListeners in the solrconfig.xml, and plugins could use the internal methods to add implicit event listeners that they need.

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: [jira] Commented: (SOLR-423) SolrRequestHandler close notification

Posted by Grant Ingersoll <gs...@apache.org>.
On Nov 28, 2007, at 5:02 PM, Ryan McKinley (JIRA) wrote:

>
> with java 1.5 this could be:
>
> for( SolrRequestHandler handler : handlers ) {
>  handler.close();
> }
>

Old habits die hard.  Especially when I am so used to typing "itco" in  
IntelliJ for the Live Template mechanism. 
  

[jira] Commented: (SOLR-423) SolrRequestHandler close notification

Posted by "Ryan McKinley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12546427 ] 

Ryan McKinley commented on SOLR-423:
------------------------------------

Aaaah that freaking interface!  So far, we have not broken API compatibility in 1.3 (though I have tried!) so I think we ought to find another way.

Off hand I see two options:

1.  Add close( SolrCore core ) to the SolrCoreAware interface
2.  Put the close method in RequestHandlerBase and have RequestHandlers.closeHandlers() check instanceof

#1 seems interesting because it would also give access to close() for QueryResponseWriters and SearchComponents

----

side note, where you have:

+ for (Iterator<SolrRequestHandler> objectIterator = handlers.values().iterator(); objectIterator.hasNext();) {
+      SolrRequestHandler handler = objectIterator.next();
+      handler.close();
+    }

with java 1.5 this could be:

for( SolrRequestHandler handler : handlers ) {
  handler.close();
}



> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-423) SolrRequestHandler close notification

Posted by "Yonik Seeley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12548514 ] 

Yonik Seeley commented on SOLR-423:
-----------------------------------

SolrEventListener is a place where we might be able to make changes because no one uses their own listeners (they probably just use Solr's builtin listeners).

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-423) SolrRequestHandler close notification

Posted by "Grant Ingersoll (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Grant Ingersoll updated SOLR-423:
---------------------------------

    Affects Version/s: 1.3
        Fix Version/s: 1.3

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>    Affects Versions: 1.3
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>             Fix For: 1.3
>
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-423) SolrRequestHandler close notification

Posted by "Grant Ingersoll (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12560388#action_12560388 ] 

Grant Ingersoll commented on SOLR-423:
--------------------------------------

Meant to follow up on this.  I think the close hook is fine.

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
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-423) SolrRequestHandler close notification

Posted by "Ryan McKinley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12546427 ] 

ryantxu edited comment on SOLR-423 at 11/28/07 2:02 PM:
--------------------------------------------------------------

Aaaah that freaking interface!  So far, we have not broken API compatibility in 1.3 (though I have tried!) so I think we ought to find another way.

Off hand I see two options:

1.  Add close( SolrCore core ) to the SolrCoreAware interface
2.  Put the close method in RequestHandlerBase and have RequestHandlers.closeHandlers() check instanceof

#1 seems interesting because it would also give access to close() for QueryResponseWriters and SearchComponents

----

side note, where you have:

{code:java}
 for (Iterator<SolrRequestHandler> objectIterator = handlers.values().iterator(); objectIterator.hasNext();) {
      SolrRequestHandler handler = objectIterator.next();
      handler.close();
   }
{code}
with java 1.5 this could be:

{code:java}
for( SolrRequestHandler handler : handlers ) {
  handler.close();
}
{code}


      was (Author: ryantxu):
    Aaaah that freaking interface!  So far, we have not broken API compatibility in 1.3 (though I have tried!) so I think we ought to find another way.

Off hand I see two options:

1.  Add close( SolrCore core ) to the SolrCoreAware interface
2.  Put the close method in RequestHandlerBase and have RequestHandlers.closeHandlers() check instanceof

#1 seems interesting because it would also give access to close() for QueryResponseWriters and SearchComponents

----

side note, where you have:

+ for (Iterator<SolrRequestHandler> objectIterator = handlers.values().iterator(); objectIterator.hasNext();) {
+      SolrRequestHandler handler = objectIterator.next();
+      handler.close();
+    }

with java 1.5 this could be:

for( SolrRequestHandler handler : handlers ) {
  handler.close();
}


  
> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>         Attachments: SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Work started: (SOLR-423) SolrRequestHandler close notification

Posted by "Grant Ingersoll (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Work on SOLR-423 started by Grant Ingersoll.

> SolrRequestHandler close notification
> -------------------------------------
>
>                 Key: SOLR-423
>                 URL: https://issues.apache.org/jira/browse/SOLR-423
>             Project: Solr
>          Issue Type: Improvement
>    Affects Versions: 1.3
>            Reporter: Grant Ingersoll
>            Assignee: Grant Ingersoll
>            Priority: Minor
>             Fix For: 1.3
>
>         Attachments: SOLR-423-CloseHook.patch, SOLR-423.patch, SOLR-423.patch
>
>
> It may be beneficial for implementations of SolrRequestHandler to be notified that the SolrCore is closing so that they can release any resources that they may have open.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.