Solr
  1. Solr
  2. SOLR-1416

reduce contention in CoreContainer#getCore()

    Details

    • Type: Improvement Improvement
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: multicore
    • Labels:
      None

      Description

      every call to CoreContainer#getCore() is synchronized . We should reduce the contention . The writes are very infrequent and reads are frequent . How about using a ReadWriterLock?

        Issue Links

          Activity

          Noble Paul created issue -
          Noble Paul made changes -
          Field Original Value New Value
          Attachment SOLR-1416.patch [ 12418831 ]
          Shalin Shekhar Mangar made changes -
          Link This issue blocks SOLR-1293 [ SOLR-1293 ]
          Shalin Shekhar Mangar made changes -
          Component/s multicore [ 12313102 ]
          Hide
          David Smiley added a comment -

          I don't think ReadWriterLock makes sense when the operation that you're doing is cheap (e.g. simple hashtable lookups). I argue you've actually slowed things down! I think ConcurrentHashMap makes the most sense.

          Just curious, did you have trepidation about this making you stop short of committing it? You were a committer when you posted it.

          Show
          David Smiley added a comment - I don't think ReadWriterLock makes sense when the operation that you're doing is cheap (e.g. simple hashtable lookups). I argue you've actually slowed things down! I think ConcurrentHashMap makes the most sense. Just curious, did you have trepidation about this making you stop short of committing it? You were a committer when you posted it.
          Hide
          Mark Miller added a comment -

          Agreed - without some credible data, this seems like the wrong change. The synch'd operation would appear to be too cheap to benefit from a reader/writer lock.

          Not sure if a ConcurrentHashMap could be used though...I think we would lose a slightly higher sync level we need?

          Show
          Mark Miller added a comment - Agreed - without some credible data, this seems like the wrong change. The synch'd operation would appear to be too cheap to benefit from a reader/writer lock. Not sure if a ConcurrentHashMap could be used though...I think we would lose a slightly higher sync level we need?
          Hide
          Erick Erickson added a comment -

          SPRING_CLEANING_2013 JIRA.
          We haven't really seen any evidence that this is a problem. As it happens once per search request I'm going to defer.

          Show
          Erick Erickson added a comment - SPRING_CLEANING_2013 JIRA. We haven't really seen any evidence that this is a problem. As it happens once per search request I'm going to defer.
          Erick Erickson made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Won't Fix [ 2 ]
          Hide
          Shalin Shekhar Mangar added a comment -

          Noble and I worked on LotsOfCores for AOL which had more than 20K cores per Solr instance. The top three factors slowing down solr for such use-cases were:

          1. Opening IndexSearcher (our use-case had a 10:1 write/read ratio) - solved by opening searcher lazily
          2. Loading/parsing IndexSchema and SolrConfig objects - solved by caching these objects
          3. Contention in CoreContainer#getCore - solved by the attached patch

          At this later date, I don't have the data to support this change but it is worth benchmarking if someone is up for it.

          Show
          Shalin Shekhar Mangar added a comment - Noble and I worked on LotsOfCores for AOL which had more than 20K cores per Solr instance. The top three factors slowing down solr for such use-cases were: Opening IndexSearcher (our use-case had a 10:1 write/read ratio) - solved by opening searcher lazily Loading/parsing IndexSchema and SolrConfig objects - solved by caching these objects Contention in CoreContainer#getCore - solved by the attached patch At this later date, I don't have the data to support this change but it is worth benchmarking if someone is up for it.
          Mark Miller made changes -
          Resolution Won't Fix [ 2 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Mark Miller added a comment -

          I reopened. This remains a good issue.

          Show
          Mark Miller added a comment - I reopened. This remains a good issue.

            People

            • Assignee:
              Unassigned
              Reporter:
              Noble Paul
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development