Solr
  1. Solr
  2. SOLR-5232

SolrCloud should distribute updates via streaming rather than buffering.

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.6, Trunk
    • Component/s: SolrCloud
    • Labels:
      None

      Description

      The current approach was never the best for SolrCloud - it was designed for a pre SolrCloud Solr - it also uses too many connections and threads - nailing that down is likely wasted effort when we should really move away from explicitly buffering docs and sending small batches per thread as we have been doing.

      1. SOLR-5232.patch
        61 kB
        Mark Miller
      2. SOLR-5232.patch
        58 kB
        Mark Miller
      3. SOLR-5232.patch
        53 kB
        Mark Miller
      4. SOLR-5232.patch
        54 kB
        Mark Miller
      5. SOLR-5232.patch
        46 kB
        Mark Miller
      6. SOLR-5232.patch
        36 kB
        Mark Miller

        Issue Links

          Activity

          Hide
          Mark Miller added a comment -

          I hacked together an initial patch that uses a ConcurrentSolrServer per url with 1 thread and a queue of 10. I originally played around with sharing them across update requests as a static, but it ends up being difficult to wait until you know the updates are accepted, so I abandoned that.

          Show
          Mark Miller added a comment - I hacked together an initial patch that uses a ConcurrentSolrServer per url with 1 thread and a queue of 10. I originally played around with sharing them across update requests as a static, but it ends up being difficult to wait until you know the updates are accepted, so I abandoned that.
          Hide
          Mark Miller added a comment -

          Another quick pass - biggest change is sharing thread executor across requests to reduce the need to spin up new threads.

          Show
          Mark Miller added a comment - Another quick pass - biggest change is sharing thread executor across requests to reduce the need to spin up new threads.
          Hide
          Erick Erickson added a comment -

          Mark:

          Just so it's recorded, there has been anecdotal evidence that the batch size of 10 we were using is a bottleneck, but there were reasons to not bump it up. Do you expect streaming to alter the throughput here?

          Show
          Erick Erickson added a comment - Mark: Just so it's recorded, there has been anecdotal evidence that the batch size of 10 we were using is a bottleneck, but there were reasons to not bump it up. Do you expect streaming to alter the throughput here?
          Hide
          Yonik Seeley added a comment -

          there has been anecdotal evidence that the batch size of 10 we were using is a bottleneck

          batch size != queue size... and it's not even clear the queue does anything unless one is somehow producing documents in a very bursty fashion.
          Streaming rather than buffering is definitely the way to go.

          Show
          Yonik Seeley added a comment - there has been anecdotal evidence that the batch size of 10 we were using is a bottleneck batch size != queue size... and it's not even clear the queue does anything unless one is somehow producing documents in a very bursty fashion. Streaming rather than buffering is definitely the way to go.
          Hide
          Mark Miller added a comment -

          This patch adds back in the retry on forwarding to leader logic (had hacked it out initially) and a new simple test for retries.

          Show
          Mark Miller added a comment - This patch adds back in the retry on forwarding to leader logic (had hacked it out initially) and a new simple test for retries.
          Hide
          Erick Erickson added a comment -

          bq: batch size != queue size

          I wasn't confusing the two, they just happen to be the same numbers. What I'm after here is something on record as to whether we expect the anecdotal evidence of faster indexing by increasing batch size to be affected by streaming, making changing the batch size question less relevant.

          I suppose with Joel's/Mark's changes to CloudSolrServer, much of the question goes away anyway, and this JIRA will then primarily affect the leader forwarding the updates to the followers.

          Of course all bets are off if a user directly uses SUSS. Seems like a "best practice" is use CloudSolrServer and if you really insist on NOT using it, then this JIRA will keep you from deadlock.

          Show
          Erick Erickson added a comment - bq: batch size != queue size I wasn't confusing the two, they just happen to be the same numbers. What I'm after here is something on record as to whether we expect the anecdotal evidence of faster indexing by increasing batch size to be affected by streaming, making changing the batch size question less relevant. I suppose with Joel's/Mark's changes to CloudSolrServer, much of the question goes away anyway, and this JIRA will then primarily affect the leader forwarding the updates to the followers. Of course all bets are off if a user directly uses SUSS. Seems like a "best practice" is use CloudSolrServer and if you really insist on NOT using it, then this JIRA will keep you from deadlock.
          Hide
          Mark Miller added a comment -

          All SolrJ clients and non SolrJ clients should fall under 'best practice' depending on your use case and desires. Using the new routing in CloudSolrServer is a possible workaround for issues that should be addressed, not worked around. Also, routing removes forwarding to leaders - you still send documents to replicas - that is still reliant on the buffer. CloudSolrServer can run also be used in a manner that does not route as an aside.

          making changing the batch size question less relevant.

          As the comments on the batch size issue indicate, changing the size is not an option we are willing to go with.

          As far as speed comparisons, someone would have to do some benchmarks to know how the new strategy holds up - it's just clearly where we have always needed to get to eventually.

          There is no similar buffer size to consider here.

          Show
          Mark Miller added a comment - All SolrJ clients and non SolrJ clients should fall under 'best practice' depending on your use case and desires. Using the new routing in CloudSolrServer is a possible workaround for issues that should be addressed, not worked around. Also, routing removes forwarding to leaders - you still send documents to replicas - that is still reliant on the buffer. CloudSolrServer can run also be used in a manner that does not route as an aside. making changing the batch size question less relevant. As the comments on the batch size issue indicate, changing the size is not an option we are willing to go with. As far as speed comparisons, someone would have to do some benchmarks to know how the new strategy holds up - it's just clearly where we have always needed to get to eventually. There is no similar buffer size to consider here.
          Hide
          Otis Gospodnetic added a comment -

          Mark Miller does that mean we can close SOLR-4956 as Won't Fix?

          Show
          Otis Gospodnetic added a comment - Mark Miller does that mean we can close SOLR-4956 as Won't Fix?
          Hide
          Mark Miller added a comment -

          Or resolve it as fixed by this if you wanted to track the issue as the performance problem. I'm fine with either approach.

          We still have to look at thread count usage with the this though - and do some general checks.

          Show
          Mark Miller added a comment - Or resolve it as fixed by this if you wanted to track the issue as the performance problem. I'm fine with either approach. We still have to look at thread count usage with the this though - and do some general checks.
          Hide
          Mark Miller added a comment -

          This patch takes care of the nocommits in the previous patch. I'll commit shortly.

          Show
          Mark Miller added a comment - This patch takes care of the nocommits in the previous patch. I'll commit shortly.
          Hide
          Shalin Shekhar Mangar added a comment -

          fyi, ShardSplitTest is much slower with this patch e.g.

          ant -Dtests.seed=8FEFBE4A48F65343 -Dtestcase=ShardSplitTest clean test-core

          Without patch: Total time: 1 minute 48 seconds
          With patch: Total time: 4 minutes 25 seconds

          Show
          Shalin Shekhar Mangar added a comment - fyi, ShardSplitTest is much slower with this patch e.g. ant -Dtests.seed=8FEFBE4A48F65343 -Dtestcase=ShardSplitTest clean test-core Without patch: Total time: 1 minute 48 seconds With patch: Total time: 4 minutes 25 seconds
          Hide
          Mark Miller added a comment -

          Hmm, lost in the noise for me because 205.66ms (that test's runtime for me) doesn't increase the total run time with tests.jvms=8.

          I'll dig into it.

          Show
          Mark Miller added a comment - Hmm, lost in the noise for me because 205.66ms (that test's runtime for me) doesn't increase the total run time with tests.jvms=8. I'll dig into it.
          Hide
          Mark Miller added a comment -

          Here is a patch with some improvements.

          I still have not gotten to the root cause of the SplitShardTest slowdown. Still digging around.

          Show
          Mark Miller added a comment - Here is a patch with some improvements. I still have not gotten to the root cause of the SplitShardTest slowdown. Still digging around.
          Hide
          Mark Miller added a comment -

          This patch address a performance issue when you were sending few docs per request to replicas (1 replica per request was really bad).

          This was because each request runner would wait 250ms when the queue was empty just in case something came along. That was huge.

          This patch waits for 0ms, which removes the issue for the few updates per request case and should not hurt the bulk docs per request case noticeably because we are using a queue of 100 anyway.

          Show
          Mark Miller added a comment - This patch address a performance issue when you were sending few docs per request to replicas (1 replica per request was really bad). This was because each request runner would wait 250ms when the queue was empty just in case something came along. That was huge. This patch waits for 0ms, which removes the issue for the few updates per request case and should not hurt the bulk docs per request case noticeably because we are using a queue of 100 anyway.
          Hide
          ASF subversion and git services added a comment -

          Commit 1533649 from Mark Miller in branch 'dev/trunk'
          [ https://svn.apache.org/r1533649 ]

          SOLR-5216: Document updates to SolrCloud can cause a distributed deadlock.
          SOLR-5232: SolrCloud should distribute updates via streaming rather than buffering.

          Show
          ASF subversion and git services added a comment - Commit 1533649 from Mark Miller in branch 'dev/trunk' [ https://svn.apache.org/r1533649 ] SOLR-5216 : Document updates to SolrCloud can cause a distributed deadlock. SOLR-5232 : SolrCloud should distribute updates via streaming rather than buffering.
          Hide
          ASF subversion and git services added a comment -

          Commit 1533652 from Mark Miller in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1533652 ]

          SOLR-5216: Document updates to SolrCloud can cause a distributed deadlock.
          SOLR-5232: SolrCloud should distribute updates via streaming rather than buffering.

          Show
          ASF subversion and git services added a comment - Commit 1533652 from Mark Miller in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1533652 ] SOLR-5216 : Document updates to SolrCloud can cause a distributed deadlock. SOLR-5232 : SolrCloud should distribute updates via streaming rather than buffering.
          Hide
          Mark Miller added a comment -

          I look forward to any further review or testing from interested parties. Resolving this for now. Let me know what you find.

          Show
          Mark Miller added a comment - I look forward to any further review or testing from interested parties. Resolving this for now. Let me know what you find.
          Hide
          Chris Geeringh added a comment -

          Mark, I have my servers updated, however is it necessary to update solrj in my indexing client? I'm not seeing the maven jar in the repository relating to build #443.

          Show
          Chris Geeringh added a comment - Mark, I have my servers updated, however is it necessary to update solrj in my indexing client? I'm not seeing the maven jar in the repository relating to build #443.
          Hide
          Chris Geeringh added a comment -

          This patch hasnt resolved the issue. 1.3 mill docs inserted and deadlock/no updates being accepted.

          I ran netstat on the servers, each of them have between 80-100 connections between each other in CLOSE_WAIT belonging to tomcat (only solr deployed)

          Show
          Chris Geeringh added a comment - This patch hasnt resolved the issue. 1.3 mill docs inserted and deadlock/no updates being accepted. I ran netstat on the servers, each of them have between 80-100 connections between each other in CLOSE_WAIT belonging to tomcat (only solr deployed)
          Hide
          Mark Miller added a comment -

          That means you were not liking seeing the same semaphore distrib lock that SOLR-5216 is about (that semaphore no longer exists) and there is something else going on with your setup.

          Show
          Mark Miller added a comment - That means you were not liking seeing the same semaphore distrib lock that SOLR-5216 is about (that semaphore no longer exists) and there is something else going on with your setup.
          Hide
          Chris Geeringh added a comment -

          OK, well as long as we're narrowing down the potential problem. The connections reported by netstat were all between the servers (not between the servers and my indexing client on different machine)

          Show
          Chris Geeringh added a comment - OK, well as long as we're narrowing down the potential problem. The connections reported by netstat were all between the servers (not between the servers and my indexing client on different machine)
          Hide
          Mark Miller added a comment -

          I think we will know a lot more as people start reporting in on their results now.

          Show
          Mark Miller added a comment - I think we will know a lot more as people start reporting in on their results now.
          Hide
          ASF subversion and git services added a comment -

          Commit 1538112 from Mark Miller in branch 'dev/trunk'
          [ https://svn.apache.org/r1538112 ]

          SOLR-5232: fix retry logic

          Show
          ASF subversion and git services added a comment - Commit 1538112 from Mark Miller in branch 'dev/trunk' [ https://svn.apache.org/r1538112 ] SOLR-5232 : fix retry logic
          Hide
          ASF subversion and git services added a comment -

          Commit 1538113 from Mark Miller in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1538113 ]

          SOLR-5232: fix retry logic

          Show
          ASF subversion and git services added a comment - Commit 1538113 from Mark Miller in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1538113 ] SOLR-5232 : fix retry logic
          Hide
          Jessica Cheng Mallet added a comment -

          Just curious--has anyone gotten a chance to run with both before and after this change to see if the throughput is improved?

          Show
          Jessica Cheng Mallet added a comment - Just curious--has anyone gotten a chance to run with both before and after this change to see if the throughput is improved?
          Hide
          Mark Miller added a comment -

          I did some tests before and after with the Lucene benchmark for Solr integration patch I have - I don't recall the exact numbers, but it was a pretty decent improvement for batching or streaming cases.

          Show
          Mark Miller added a comment - I did some tests before and after with the Lucene benchmark for Solr integration patch I have - I don't recall the exact numbers, but it was a pretty decent improvement for batching or streaming cases.
          Hide
          Mark Miller added a comment -

          I do think the per request time may have gone up a bit (and we might be able to improve that), but with a tradeoff that batch or streaming updates are much, much faster.

          Show
          Mark Miller added a comment - I do think the per request time may have gone up a bit (and we might be able to improve that), but with a tradeoff that batch or streaming updates are much, much faster.
          Hide
          Mark Miller added a comment -

          Some more real world experience - the old system of internally sending around batches of 10 docs was horribly inefficient and a major performance limiter. The only way this might not be the case was if you were using client side hashing and no replicas. Batching with multiple threads is the key to performance with SolrCloud and the internal batch by 10 would just decimate the performance no matter the size the user batched - even with no replicas and just internal forwarding. This change unlocked that performance bottleneck and is at least many times faster in some cases.

          Show
          Mark Miller added a comment - Some more real world experience - the old system of internally sending around batches of 10 docs was horribly inefficient and a major performance limiter. The only way this might not be the case was if you were using client side hashing and no replicas. Batching with multiple threads is the key to performance with SolrCloud and the internal batch by 10 would just decimate the performance no matter the size the user batched - even with no replicas and just internal forwarding. This change unlocked that performance bottleneck and is at least many times faster in some cases.

            People

            • Assignee:
              Mark Miller
              Reporter:
              Mark Miller
            • Votes:
              4 Vote for this issue
              Watchers:
              20 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development