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 "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org> on 2008/12/16 07:42:44 UTC

[jira] Created: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

Thread safety issue in SolrCore.infoRegistry
--------------------------------------------

                 Key: SOLR-918
                 URL: https://issues.apache.org/jira/browse/SOLR-918
             Project: Solr
          Issue Type: Bug
            Reporter: Shalin Shekhar Mangar
            Priority: Minor
             Fix For: 1.4


SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.

We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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


[jira] Updated: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

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

Shalin Shekhar Mangar updated SOLR-918:
---------------------------------------

    Attachment: SOLR-918.patch

Patch to change LinkedHashMap to ConcurrentHashMap. I plan to commit shortly.

> Thread safety issue in SolrCore.infoRegistry
> --------------------------------------------
>
>                 Key: SOLR-918
>                 URL: https://issues.apache.org/jira/browse/SOLR-918
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Shalin Shekhar Mangar
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-918.patch
>
>
> SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.
> We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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


[jira] Updated: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

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

Shalin Shekhar Mangar updated SOLR-918:
---------------------------------------

    Attachment: SOLR-918.patch

# Removed synchronization for iterating infoRegistry from stats.jsp and PluginInfoHandler
# Updated Javadocs for SolrCore#getInfoRegistry to:
{code}
+   * Returns a Map of name vs SolrInfoMBean objects. The returned map is an instance of
+   * a ConcurrentHashMap and therefore no synchronization is needed for putting, removing
+   * or iterating over it.
+   *
{code}

I plan to commit shortly.

> Thread safety issue in SolrCore.infoRegistry
> --------------------------------------------
>
>                 Key: SOLR-918
>                 URL: https://issues.apache.org/jira/browse/SOLR-918
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Shalin Shekhar Mangar
>            Assignee: Shalin Shekhar Mangar
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-918.patch, SOLR-918.patch
>
>
> SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.
> We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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


[jira] Commented: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

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

Yonik Seeley commented on SOLR-918:
-----------------------------------

bq. Is it ok to let it be a ConcurrentHashMap or should we make it a synchronized LinkedHashMap

Either should be fine... new components are added so infrequently and checked so infrequently that there won't be a performance difference.

If it's changed to synchronized LinkedHashMap, it should be noted in the javadoc for SolrCore.getInfoRegistry() (which is new in 1.3) that it's a synchronized Map and thus users need to synchronize during iteration.  This synchronization is currently being done by all users of this API since it previously was a synchronized map (before the synchronization was mistakenly removed).

If we stay with ConcurrentHashMap (and hence don't care about order), then the current synchronization while iterating should probably be removed too.

> Thread safety issue in SolrCore.infoRegistry
> --------------------------------------------
>
>                 Key: SOLR-918
>                 URL: https://issues.apache.org/jira/browse/SOLR-918
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Shalin Shekhar Mangar
>            Assignee: Shalin Shekhar Mangar
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-918.patch
>
>
> SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.
> We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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


[jira] Commented: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12658016#action_12658016 ] 

Shalin Shekhar Mangar commented on SOLR-918:
--------------------------------------------

Alright. When JMX is enabled, it uses the JmxMonitoredMap which extends ConcurrentHashMap so order is not maintained anyway. Let's stick with ConcurrentHashMap, I'll scan through the code and remove all synchronization during iteration and update the javadocs.

> Thread safety issue in SolrCore.infoRegistry
> --------------------------------------------
>
>                 Key: SOLR-918
>                 URL: https://issues.apache.org/jira/browse/SOLR-918
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Shalin Shekhar Mangar
>            Assignee: Shalin Shekhar Mangar
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-918.patch
>
>
> SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.
> We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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


[jira] Commented: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12657661#action_12657661 ] 

Shalin Shekhar Mangar commented on SOLR-918:
--------------------------------------------

It was a HashMap in the SOLR-256 commit but I think it was changed (by me?) to a LinkedHashMap because some users complained that the order of things in stats pages is not the same anymore. Is it ok to let it be a ConcurrentHashMap or should we make it a synchronized LinkedHashMap?

> Thread safety issue in SolrCore.infoRegistry
> --------------------------------------------
>
>                 Key: SOLR-918
>                 URL: https://issues.apache.org/jira/browse/SOLR-918
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Shalin Shekhar Mangar
>            Assignee: Shalin Shekhar Mangar
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-918.patch
>
>
> SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.
> We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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


[jira] Resolved: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

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

Shalin Shekhar Mangar resolved SOLR-918.
----------------------------------------

    Resolution: Fixed
      Assignee: Shalin Shekhar Mangar

Committed revision 727370.

> Thread safety issue in SolrCore.infoRegistry
> --------------------------------------------
>
>                 Key: SOLR-918
>                 URL: https://issues.apache.org/jira/browse/SOLR-918
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Shalin Shekhar Mangar
>            Assignee: Shalin Shekhar Mangar
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-918.patch
>
>
> SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.
> We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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


[jira] Commented: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

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

Yonik Seeley commented on SOLR-918:
-----------------------------------

Hmmm, it used to be a synchronized map.
Looks like this bug was introduced back in one of those mega-changes, SOLR-215


> Thread safety issue in SolrCore.infoRegistry
> --------------------------------------------
>
>                 Key: SOLR-918
>                 URL: https://issues.apache.org/jira/browse/SOLR-918
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Shalin Shekhar Mangar
>            Assignee: Shalin Shekhar Mangar
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-918.patch
>
>
> SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.
> We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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


[jira] Commented: (SOLR-918) Thread safety issue in SolrCore.infoRegistry

Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12658022#action_12658022 ] 

Shalin Shekhar Mangar commented on SOLR-918:
--------------------------------------------

Committed revision 727944.

> Thread safety issue in SolrCore.infoRegistry
> --------------------------------------------
>
>                 Key: SOLR-918
>                 URL: https://issues.apache.org/jira/browse/SOLR-918
>             Project: Solr
>          Issue Type: Bug
>            Reporter: Shalin Shekhar Mangar
>            Assignee: Shalin Shekhar Mangar
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-918.patch, SOLR-918.patch
>
>
> SolrCore.infoRegistry is used for keeping a reference to the SolrInfoMBean objects. When JMX is disabled, it is initialized to a LinkedHashMap which is not synchronized.
> We can change this to a ConcurrentHashMap instead of a LinkedHashMap.

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