Solr
  1. Solr
  2. SOLR-2705

On reload, IndexWriterProvider holds onto the initial SolrCore it was created with

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      dug up by Yury Kats in SOLR-2326

      The IWP needs to be updated to ref the new core on reload - else if some settings have changed (eg newIndexDir change caused by replication), the IndexWriter will be incorrectly reopened.

      This should really only be an issue when replication and core reloads are involved for the most part.

      1. SOLR-2705.patch
        12 kB
        Mark Miller
      2. SOLR-2705.patch
        14 kB
        Mark Miller

        Activity

        Hide
        Mark Miller added a comment -

        Test case + fix

        Show
        Mark Miller added a comment - Test case + fix
        Hide
        Yury Kats added a comment -

        The problem is reproducible on the stock multicore example

        Show
        Yury Kats added a comment - The problem is reproducible on the stock multicore example configure multicore's ReplicationHandler to replicate after startup and commit launch multicore example index some example docs ask for commits: http://localhost:8983/solr/core0/replication?command=commits reload core0: http://localhost:8983/solr/admin/cores?action=RELOAD&core=core0 ask for commits again: http://localhost:8983/solr/core0/replication?command=commits observe no commits being returned after reload.
        Hide
        Mark Miller added a comment -

        I'm going to pop this one in now. Ugly little bug.

        Show
        Mark Miller added a comment - I'm going to pop this one in now. Ugly little bug.
        Hide
        Mark Miller added a comment -

        Committed - Yury, can you confirm this works for you use case now?

        Show
        Mark Miller added a comment - Committed - Yury, can you confirm this works for you use case now?
        Hide
        Yury Kats added a comment -

        Thanks, I will try.
        Looking at the patch, it's not clear to me whether the reloaded core would return proper Commits right after it's been reloaded, or only after the subsequent commit.
        Can Junit check for indexversion/commits to be same right before and right after coreReload?

        Show
        Yury Kats added a comment - Thanks, I will try. Looking at the patch, it's not clear to me whether the reloaded core would return proper Commits right after it's been reloaded, or only after the subsequent commit. Can Junit check for indexversion/commits to be same right before and right after coreReload?
        Hide
        Yury Kats added a comment -

        First quick test looks good!

        Show
        Yury Kats added a comment - First quick test looks good!
        Hide
        Mark Miller added a comment -

        Sure, likely easy enough to add.

        The current test fails without the fix because the SolrCore is changed on the reload, and replication will sometimes kick off the creation of a new index writer. If it does this using the old SolrCore's settings/fields, as was happening, it might not even be writing to the right index! And it's DeletionPolicy will be the incorrect instance from the old core, and... When the new writer is made, it needs it's info to come from the new SolrCore of course. This bug was nasty - unfortunately we didn't have a test that could tickle it.

        So in this test, after the reload, if you add any further documents, they won't show up in any searches, even after commit.

        So the test adds 10 docs, ensures they are in the index, then adds 2 more docs, and ensures 12 docs are in the index - without the fix it would find 10 rather than 12 - until you restart the server.

        Show
        Mark Miller added a comment - Sure, likely easy enough to add. The current test fails without the fix because the SolrCore is changed on the reload, and replication will sometimes kick off the creation of a new index writer. If it does this using the old SolrCore's settings/fields, as was happening, it might not even be writing to the right index! And it's DeletionPolicy will be the incorrect instance from the old core, and... When the new writer is made, it needs it's info to come from the new SolrCore of course. This bug was nasty - unfortunately we didn't have a test that could tickle it. So in this test, after the reload, if you add any further documents, they won't show up in any searches, even after commit. So the test adds 10 docs, ensures they are in the index, then adds 2 more docs, and ensures 12 docs are in the index - without the fix it would find 10 rather than 12 - until you restart the server.
        Hide
        Yury Kats added a comment -

        Tested some more, all is well. Thanks a lot, Mark. This is outstanding support and turn around time from a committer. Hats off!

        I saw in the patch that you've added getIndexversion and getCommits, but haven't used them. Would be a shame not to put them to work. It would also ensure that replication works right after a RELOAD, which can be a case in my app, so I'm in favor of testing that angle as well.

        Show
        Yury Kats added a comment - Tested some more, all is well. Thanks a lot, Mark. This is outstanding support and turn around time from a committer. Hats off! I saw in the patch that you've added getIndexversion and getCommits, but haven't used them. Would be a shame not to put them to work. It would also ensure that replication works right after a RELOAD, which can be a case in my app, so I'm in favor of testing that angle as well.
        Hide
        Mark Miller added a comment -

        Hmm...so that improved things, but it's not quite the right fix. If multiple SolrCores are going to share the IndexWriterProvider for any length of time, it cannot hold a ref to a SolrCore. It's rare to have multiple reloaded SolrCores up at the same time with a Writer (I know because it wasn't even possible a short time ago without running into IndexLock exceptions), but this should now be fine to do. So a better solution is to pass the SolrCore to the methods of IndexWriterProver.

        Show
        Mark Miller added a comment - Hmm...so that improved things, but it's not quite the right fix. If multiple SolrCores are going to share the IndexWriterProvider for any length of time, it cannot hold a ref to a SolrCore. It's rare to have multiple reloaded SolrCores up at the same time with a Writer (I know because it wasn't even possible a short time ago without running into IndexLock exceptions), but this should now be fine to do. So a better solution is to pass the SolrCore to the methods of IndexWriterProver.
        Hide
        Mark Miller added a comment -

        I saw in the patch that you've added getIndexversion and getCommits, but haven't used them.

        Right - they where good for debugging and I figured I would use them to investigate SOLR-2326.

        Can Junit check for indexversion/commits to be same right before and right after coreReload?

        I've committed this check.

        Show
        Mark Miller added a comment - I saw in the patch that you've added getIndexversion and getCommits, but haven't used them. Right - they where good for debugging and I figured I would use them to investigate SOLR-2326 . Can Junit check for indexversion/commits to be same right before and right after coreReload? I've committed this check.
        Hide
        Mark Miller added a comment -

        Thanks Yury!

        Show
        Mark Miller added a comment - Thanks Yury!

          People

          • Assignee:
            Mark Miller
            Reporter:
            Mark Miller
          • Votes:
            2 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development