Solr
  1. Solr
  2. SOLR-4608

Update Log replay should use the default processor chain

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.1, 4.2
    • Fix Version/s: 4.2.1, 4.3, Trunk
    • Component/s: SolrCloud
    • Labels:
      None

      Description

      If a processor chain is used with custom processors,
      they are not used in case of node failure during log replay.

      Here is the code:

      UpdateLog.java
          public void doReplay(TransactionLog translog) {
            try {
              loglog.warn("Starting log replay " + translog + " active="+activeLog + " starting pos=" + recoveryInfo.positionOfStart);
      
              tlogReader = translog.getReader(recoveryInfo.positionOfStart);
      
              // NOTE: we don't currently handle a core reload during recovery.  This would cause the core
              // to change underneath us.
      
              // TODO: use the standard request factory?  We won't get any custom configuration instantiating this way.
              RunUpdateProcessorFactory runFac = new RunUpdateProcessorFactory();
              DistributedUpdateProcessorFactory magicFac = new DistributedUpdateProcessorFactory();
              runFac.init(new NamedList());
              magicFac.init(new NamedList());
      
              UpdateRequestProcessor proc = magicFac.getInstance(req, rsp, runFac.getInstance(req, rsp, null));
      

      I think this is a big issue, because a lot of people will discover it when a node will crash in the best case... and I think it's too late.

      It means to me that processor chains are not usable with Solr Cloud currently.

      1. SOLR-4608.patch
        4 kB
        Yonik Seeley

        Activity

        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Yonik Seeley
        http://svn.apache.org/viewvc?view=revision&revision=1459427

        SOLR-4608: use default update processor chain during log replay and peersync

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Yonik Seeley http://svn.apache.org/viewvc?view=revision&revision=1459427 SOLR-4608 : use default update processor chain during log replay and peersync
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] Yonik Seeley
        http://svn.apache.org/viewvc?view=revision&revision=1459424

        SOLR-4608: use default update processor chain during log replay and peersync

        Show
        Commit Tag Bot added a comment - [trunk commit] Yonik Seeley http://svn.apache.org/viewvc?view=revision&revision=1459424 SOLR-4608 : use default update processor chain during log replay and peersync
        Hide
        Mark Miller added a comment -

        Yonik Seeley, this ready to be committed? I'd like to toss it in 4.2.1.

        Show
        Mark Miller added a comment - Yonik Seeley , this ready to be committed? I'd like to toss it in 4.2.1.
        Hide
        ludovic Boutros added a comment -

        You're right Yonik, it works now. Thanks.
        Do you think the patch could be committed in the different branches ?
        If I can help, just ask.

        Show
        ludovic Boutros added a comment - You're right Yonik, it works now. Thanks. Do you think the patch could be committed in the different branches ? If I can help, just ask.
        Hide
        Yonik Seeley added a comment -

        Anything after the DistributedUpdateProcessor will not be applied, right ?

        Everything before the distributed update processor will be applied before buffering, and everything after should be applied while replaying.

        Show
        Yonik Seeley added a comment - Anything after the DistributedUpdateProcessor will not be applied, right ? Everything before the distributed update processor will be applied before buffering, and everything after should be applied while replaying.
        Hide
        ludovic Boutros added a comment -

        Anything after the DistributedUpdateProcessor will not be applied, right ?

        Do I need to create one default processor chain with my custom processor before the DistributedUpdateProcessor, and the real one used by the update handler with my custom processor after the DistributedUpdateProcessor ?

        Show
        ludovic Boutros added a comment - Anything after the DistributedUpdateProcessor will not be applied, right ? Do I need to create one default processor chain with my custom processor before the DistributedUpdateProcessor, and the real one used by the update handler with my custom processor after the DistributedUpdateProcessor ?
        Hide
        Mark Miller added a comment -

        Report back soon and hopefully we can get this in 4.2.1.

        Show
        Mark Miller added a comment - Report back soon and hopefully we can get this in 4.2.1.
        Hide
        Yonik Seeley added a comment -

        Here's a patch that uses the default chain for both log replaying and peer sync replaying.

        Show
        Yonik Seeley added a comment - Here's a patch that uses the default chain for both log replaying and peer sync replaying.
        Hide
        ludovic Boutros added a comment -

        Thanks Mark and Yonik.

        Yonik, could you please post the code of this change ?
        I could try to patch the 4.1/4.2 branches and then test it.

        Show
        ludovic Boutros added a comment - Thanks Mark and Yonik. Yonik, could you please post the code of this change ? I could try to patch the 4.1/4.2 branches and then test it.
        Hide
        Yonik Seeley added a comment - - edited

        Hmmm, it appears that making this change causes RecoveryZkTest to fail for some reason. Not sure why yet...

        edit: actually, it looks like this test is failing on a clean checkout of trunk too - so probably not related to this.

        Show
        Yonik Seeley added a comment - - edited Hmmm, it appears that making this change causes RecoveryZkTest to fail for some reason. Not sure why yet... edit: actually, it looks like this test is failing on a clean checkout of trunk too - so probably not related to this.
        Hide
        Yonik Seeley added a comment -

        Right, we should use the default chain (although only processors at or after the distrib update processor factory - just like non-leaders do).
        Normally, custom processors should go before the distributed processor, so this shouldn't affect most people.

        Show
        Yonik Seeley added a comment - Right, we should use the default chain (although only processors at or after the distrib update processor factory - just like non-leaders do). Normally, custom processors should go before the distributed processor, so this shouldn't affect most people.

          People

          • Assignee:
            Yonik Seeley
            Reporter:
            ludovic Boutros
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development