Solr
  1. Solr
  2. SOLR-6137

Managed Schema / Schemaless and SolrCloud concurrency issues

    Details

      Description

      This is a follow up to a message on the mailing list, linked here: http://mail-archives.apache.org/mod_mbox/lucene-dev/201406.mbox/%3CCAKfebOOcMeVEb010SsdcH8nta%3DyonMK5R7dSFOsbJ_tnre0O7w%40mail.gmail.com%3E

      The Managed Schema integration with SolrCloud seems pretty limited.

      The issue I'm running into is variants of the issue that schema changes are not pushed to all shards/replicas synchronously. So, for example, I can make the following two requests:
      1) add a field to the collection on server1 using the Schema API
      2) add a document with the new field, the document is routed to a core on server2

      Then, there appears to be a race between when the document is processed by the core on server2 and when the core on server2, via the ZkIndexSchemaReader, gets the new schema. If the document is processed first, I get a 400 error because the field doesn't exist. This is easily reproducible by adding a sleep to the ZkIndexSchemaReader's processing.

      I hit a similar issue with Schemaless: the distributed request handler sends out the document updates, but there is no guarantee that the other shards/replicas see the schema changes made by the update.chain.

      Another issue I noticed today: making multiple schema API calls concurrently can block; that is, one may get through and the other may infinite loop.

      So, for reference, the issues include:
      1) Schema API changes return success before all cores are updated; subsequent calls attempting to use new schema may fail
      2) Schemaless changes may fail on replicas/other shards for the same reason
      3) Concurrent Schema API changes may block

      From Steve Rowe on the mailing list:

      For Schema API users, delaying a couple of seconds after adding fields before using them should workaround this problem. While not ideal, I think schema field additions are rare enough in the Solr collection lifecycle that this is not a huge problem.

      For schemaless users, the picture is worse, as you noted. Immediate distribution of documents triggering schema field addition could easily prove problematic. Maybe we need a schema update blocking mode, where after the ZK schema node watch is triggered, all new request processing is halted until the schema is finished downloading/parsing/swapping out? (Such a mode should help Schema API users too.)

      1. SOLR-6137.patch
        28 kB
        Steve Rowe
      2. SOLR-6137.patch
        27 kB
        Gregory Chanan
      3. SOLR-6137v2.patch
        25 kB
        Gregory Chanan
      4. SOLR-6137v3.patch
        5 kB
        Gregory Chanan
      5. SOLR-6137v4.patch
        25 kB
        Gregory Chanan

        Activity

        Hide
        Gregory Chanan added a comment -

        The Schema API blocking mode is an interesting idea, I'd want to think more about that.

        In some sense, the schemaless issue seems easier to solve than the Schema API issue. This is because if we run all (or more) of the update chain, instead of just skipping to the distributed update handler on the forwarded nodes, we could have all the cores apply the schema changes, so we are guaranteed of having the correct schema on each core. We'd need to be smarter about trying to update the schema in ZK (as I noted above, concurrent schema changes may fail currently). But that doesn't seem impossible.

        The Schema API issue does seem more difficult. A blocking mode could work in theory, though I guess one complication is you need to wait for all the cores that use the config, not just all the cores of the collection. Although, perhaps we should just throw in some checks that only one collection is using a certain managed schema config at a time; it may make the logic easier and it seems very unlikely the user actually wants to use the same schema for multiple collections (I did that myself the first time before realizing why it didn't make any sense).

        As Steve noted above, a blocking mode could be used by the schemaless functionality as well, instead of what I wrote above.

        Show
        Gregory Chanan added a comment - The Schema API blocking mode is an interesting idea, I'd want to think more about that. In some sense, the schemaless issue seems easier to solve than the Schema API issue. This is because if we run all (or more) of the update chain, instead of just skipping to the distributed update handler on the forwarded nodes, we could have all the cores apply the schema changes, so we are guaranteed of having the correct schema on each core. We'd need to be smarter about trying to update the schema in ZK (as I noted above, concurrent schema changes may fail currently). But that doesn't seem impossible. The Schema API issue does seem more difficult. A blocking mode could work in theory, though I guess one complication is you need to wait for all the cores that use the config, not just all the cores of the collection. Although, perhaps we should just throw in some checks that only one collection is using a certain managed schema config at a time; it may make the logic easier and it seems very unlikely the user actually wants to use the same schema for multiple collections (I did that myself the first time before realizing why it didn't make any sense). As Steve noted above, a blocking mode could be used by the schemaless functionality as well, instead of what I wrote above.
        Hide
        Yonik Seeley added a comment -

        In some sense, the schemaless issue seems easier to solve than the Schema API issue. This is because if we run all (or more) of the update chain, instead of just skipping to the distributed update handler on the forwarded nodes, we could have all the cores apply the schema changes, so we are guaranteed of having the correct schema on each core.

        Right. Schemaless should be a non-issue. The type-guessing logic should be run on replicas as well. If the replica hasn't seen the change yet, then it will guess the same type, and try to add it to the schema. It should fail due to optimistic locking and the fact the leader already added it, re-read the schema, and then successfully find the field. It's the same case as a single node with multiple threads both encountering the new field at around the same time.

        Although the schema API needs a blocking mode, no blocking mode should be added to schemaless... that's what the optimistic concurrency is for.

        Show
        Yonik Seeley added a comment - In some sense, the schemaless issue seems easier to solve than the Schema API issue. This is because if we run all (or more) of the update chain, instead of just skipping to the distributed update handler on the forwarded nodes, we could have all the cores apply the schema changes, so we are guaranteed of having the correct schema on each core. Right. Schemaless should be a non-issue. The type-guessing logic should be run on replicas as well. If the replica hasn't seen the change yet, then it will guess the same type, and try to add it to the schema. It should fail due to optimistic locking and the fact the leader already added it, re-read the schema, and then successfully find the field. It's the same case as a single node with multiple threads both encountering the new field at around the same time. Although the schema API needs a blocking mode, no blocking mode should be added to schemaless... that's what the optimistic concurrency is for.
        Hide
        Steve Rowe added a comment -

        making multiple schema API calls concurrently can block; that is, one may get through and the other may infinite loop.

        Alexey Serba mentioned that problem to me a while back. IIRC this is a bug in the optimistic concurrency implementation. I'll dig up his info and make an issue.

        Schema API changes return success before all cores are updated; subsequent calls attempting to use new schema may fail

        One way to fix this may be to return the schema ZK node version from Schema API modification methods, and add a request param requiring a minimum schema ZK node version prior to binding a schema snapshot to the request.

        no blocking mode should be added to schemaless... that's what the optimistic concurrency is for.

        Right, thanks Yonik, I'd forgotten optimistic concurrency's part in the schemaless design.

        Show
        Steve Rowe added a comment - making multiple schema API calls concurrently can block; that is, one may get through and the other may infinite loop. Alexey Serba mentioned that problem to me a while back. IIRC this is a bug in the optimistic concurrency implementation. I'll dig up his info and make an issue. Schema API changes return success before all cores are updated; subsequent calls attempting to use new schema may fail One way to fix this may be to return the schema ZK node version from Schema API modification methods, and add a request param requiring a minimum schema ZK node version prior to binding a schema snapshot to the request. no blocking mode should be added to schemaless... that's what the optimistic concurrency is for. Right, thanks Yonik, I'd forgotten optimistic concurrency's part in the schemaless design.
        Hide
        Steve Rowe added a comment -

        making multiple schema API calls concurrently can block; that is, one may get through and the other may infinite loop.

        Alexey Serba mentioned that problem to me a while back. IIRC this is a bug in the optimistic concurrency implementation. I'll dig up his info and make an issue.

        SOLR-6145

        Show
        Steve Rowe added a comment - making multiple schema API calls concurrently can block; that is, one may get through and the other may infinite loop. Alexey Serba mentioned that problem to me a while back. IIRC this is a bug in the optimistic concurrency implementation. I'll dig up his info and make an issue. SOLR-6145
        Hide
        Yonik Seeley added a comment -

        One way to fix this may be to return the schema ZK node version from Schema API modification methods, and add a request param requiring a minimum schema ZK node version prior to binding a schema snapshot to the request.

        That's an interesting idea, but it would then require users of the schema API to add versions to all their subsequent update requests (and no clear idea when they could stop, and it gets even more interestnig with multiple update clients).
        An alternative would be to proactively check if there is a new schema version if we fail to find a field. The overhead shouldn't be an issue if we would otherwise fail the request.

        We should be careful to only do this check when necessary. For example, it seems like that queries for non-existent fields should not normally trigger a ZK lookup (and the schema only changes with the IndexSearcher anyway). But then again, a delete by query should check (it's indexing side, and one would want to be able to add a field, add a doc with that field, then do a DBQ with that field). It's context specific - tricky stuff.

        Show
        Yonik Seeley added a comment - One way to fix this may be to return the schema ZK node version from Schema API modification methods, and add a request param requiring a minimum schema ZK node version prior to binding a schema snapshot to the request. That's an interesting idea, but it would then require users of the schema API to add versions to all their subsequent update requests (and no clear idea when they could stop, and it gets even more interestnig with multiple update clients). An alternative would be to proactively check if there is a new schema version if we fail to find a field. The overhead shouldn't be an issue if we would otherwise fail the request. We should be careful to only do this check when necessary. For example, it seems like that queries for non-existent fields should not normally trigger a ZK lookup (and the schema only changes with the IndexSearcher anyway). But then again, a delete by query should check (it's indexing side, and one would want to be able to add a field, add a doc with that field, then do a DBQ with that field). It's context specific - tricky stuff.
        Hide
        Gregory Chanan added a comment -

        This is indeed tricky stuff. Consider too if the Schema API was expanded to allow the full range of actions you could do to an unmanaged schema with solr down, i.e. removing fields or changing types. Then, only checking on non-existing fields wouldn't work.

        There's also the case of the same field being added simultaneously on different nodes with different types via schemaless. I haven't actually tested that, but I bet it would hang like SOLR-6145...

        How about something like this for schemaless (I haven't totally thought this through, I'll take a closer look tomorrow to see if this is feasible):
        As I suggested before, we run more of the update.chain so the fields are added on each core. If the the update to ZK fails because of version mismatch, we download the new schema and try to apply our changes again. If the field already exists with the correct type, we don't need to do anything. If the field exists with the wrong type, we throw an exception and the update should fail (which seems correct, because that would happen if the updates were not done simultaneously).

        Show
        Gregory Chanan added a comment - This is indeed tricky stuff. Consider too if the Schema API was expanded to allow the full range of actions you could do to an unmanaged schema with solr down, i.e. removing fields or changing types. Then, only checking on non-existing fields wouldn't work. There's also the case of the same field being added simultaneously on different nodes with different types via schemaless. I haven't actually tested that, but I bet it would hang like SOLR-6145 ... How about something like this for schemaless (I haven't totally thought this through, I'll take a closer look tomorrow to see if this is feasible): As I suggested before, we run more of the update.chain so the fields are added on each core. If the the update to ZK fails because of version mismatch, we download the new schema and try to apply our changes again. If the field already exists with the correct type, we don't need to do anything. If the field exists with the wrong type, we throw an exception and the update should fail (which seems correct, because that would happen if the updates were not done simultaneously).
        Hide
        Yonik Seeley added a comment -

        Gregory, the schemaless case is covered I think... we either re-run the type guessing logic on replicas, or pass in a requireSchema=<version> parameter that would be checked by the replicas. Races between different nodes are handled via optimistic concurrency.

        Show
        Yonik Seeley added a comment - Gregory, the schemaless case is covered I think... we either re-run the type guessing logic on replicas, or pass in a requireSchema=<version> parameter that would be checked by the replicas. Races between different nodes are handled via optimistic concurrency.
        Hide
        Gregory Chanan added a comment -

        Here's a patch that addresses the issue with schemaless in a distributed environment.

        Overview:

        • Fixes an NPE issue if there are no dynamic fields and the schema is reloaded
        • The AddSchemaFieldsUpdateProcessorFactory now implements UpdateRequestProcessorFactory.RunAlways so we can guarantee that the schema additions happen on every core
        • The AddSchemaFieldsUpdateProcessor is careful about not updating the to the latest schema while it is processing. Without this, it can detect that the field exists from the latest schema, but the request's schema won't be updated and an unknown field error will occur. There may be a more efficient way to do this by making sure the request schema is updated, but the above approach should work
        • Adds a test that fails without the patch
        Show
        Gregory Chanan added a comment - Here's a patch that addresses the issue with schemaless in a distributed environment. Overview: Fixes an NPE issue if there are no dynamic fields and the schema is reloaded The AddSchemaFieldsUpdateProcessorFactory now implements UpdateRequestProcessorFactory.RunAlways so we can guarantee that the schema additions happen on every core The AddSchemaFieldsUpdateProcessor is careful about not updating the to the latest schema while it is processing. Without this, it can detect that the field exists from the latest schema, but the request's schema won't be updated and an unknown field error will occur. There may be a more efficient way to do this by making sure the request schema is updated, but the above approach should work Adds a test that fails without the patch
        Hide
        Gregory Chanan added a comment -

        Forgot to mention, the above patch assumes SOLR-6180 is applied.

        Show
        Gregory Chanan added a comment - Forgot to mention, the above patch assumes SOLR-6180 is applied.
        Hide
        Gregory Chanan added a comment -

        v2 patch: test handles setting the schema file in a standard way.

        Show
        Gregory Chanan added a comment - v2 patch: test handles setting the schema file in a standard way.
        Hide
        Gregory Chanan added a comment -

        Here's a new version of the patch. The previous one is dependent on javabin serialization. In particular, the javabin serialization preserves the types of field values, whereas XML does not. This means that if the parsing/schema addition happen before the distributed phase and we use XML serialization, the types can switch during the distributed phase and the field types may end up incorrect.

        Instead, a better solution seems to be to move the parsing/schema addition to after the distributed phase. That way, we'll run the parsing on the replicas and the types will be correct.

        Show
        Gregory Chanan added a comment - Here's a new version of the patch. The previous one is dependent on javabin serialization. In particular, the javabin serialization preserves the types of field values, whereas XML does not. This means that if the parsing/schema addition happen before the distributed phase and we use XML serialization, the types can switch during the distributed phase and the field types may end up incorrect. Instead, a better solution seems to be to move the parsing/schema addition to after the distributed phase. That way, we'll run the parsing on the replicas and the types will be correct.
        Hide
        Gregory Chanan added a comment -

        One small issue I noticed is that there is a race between parsing and schema addition. The AddSchemaFieldsUpdateProcessor handles this by only working on a fixed schema, so the schema doesn't change underneath it. If it decides on a schema addition and that fails (because another addition beat it), it will grab the latest schema and retry. But the parsers don't do that so the core's schema can change in the middle of parsing. It may make sense to defend against that by moving the retry code from the AddSchemaFieldsUpdateProcessor to some processor that runs before all the parsers. The downside is if the schema addition fails, you have to rerun all the parsers, but that may be a minor concern.

        This may not actually matter. Consider the case tested at the end of the test: two documents are simultaneously inserted with the same field having a Long and Date value. Assume the Date wins the schema "race" and is updated first. While parsing the Long, each parser may see the schema as having a date field or no field. If a valid parser (that is, one that can modify the field value) sees a date field, it won't do any modifications because shouldMutate will fail, leaving the object in whatever state the serializer left it (either Long or String). If it sees no field, it will mutate the object to create a Long object. In either case, we should get an error at the point we actually create the lucene document, because neither a Long nor String-representation-of-a-long can be stored in a Date field. This is pretty difficult to reason about though, it may be better to just do the above. But I would prefer to tackle that in another jira.

        Show
        Gregory Chanan added a comment - One small issue I noticed is that there is a race between parsing and schema addition. The AddSchemaFieldsUpdateProcessor handles this by only working on a fixed schema, so the schema doesn't change underneath it. If it decides on a schema addition and that fails (because another addition beat it), it will grab the latest schema and retry. But the parsers don't do that so the core's schema can change in the middle of parsing. It may make sense to defend against that by moving the retry code from the AddSchemaFieldsUpdateProcessor to some processor that runs before all the parsers. The downside is if the schema addition fails, you have to rerun all the parsers, but that may be a minor concern. This may not actually matter. Consider the case tested at the end of the test: two documents are simultaneously inserted with the same field having a Long and Date value. Assume the Date wins the schema "race" and is updated first. While parsing the Long, each parser may see the schema as having a date field or no field. If a valid parser (that is, one that can modify the field value) sees a date field, it won't do any modifications because shouldMutate will fail, leaving the object in whatever state the serializer left it (either Long or String). If it sees no field, it will mutate the object to create a Long object. In either case, we should get an error at the point we actually create the lucene document, because neither a Long nor String-representation-of-a-long can be stored in a Date field. This is pretty difficult to reason about though, it may be better to just do the above. But I would prefer to tackle that in another jira.
        Hide
        Gregory Chanan added a comment -

        attached the wrong patch previously.

        Show
        Gregory Chanan added a comment - attached the wrong patch previously.
        Hide
        Steve Rowe added a comment -

        Patch against current trunk, along with AddSchemaFieldsUpdateProcessorFactory.java.svnpatch.rej, the rejected elements of Gregory Chanan's v4 patch - it looks like the v4 patch was against an earlier version of the patch or something, since the removed lines don't exist on trunk. The patch is my attempt at merging current trunk with the additions from the v4 patch.

        One problem blocking compilation: oldSchema's update lock is pulled from IndexSchema.getSchemaUpdateLock(), which doesn't exist (that method is on ManagedIndexSchema) - in my patch, I've cast to oldSchema to ManagedIndexSchema to make that compile, but maybe the intent was to move getSchemaUpdateLock() to IndexSchema? (That's not part of the v4 patch, maybe git add is needed?)

        Also, Gregory, when you post patches on JIRA issues, please use the same name for each iteration of your patch, rather than adding "vN", where N increases with each patch version - JIRA will gray out older versions.

        Show
        Steve Rowe added a comment - Patch against current trunk, along with AddSchemaFieldsUpdateProcessorFactory.java.svnpatch.rej , the rejected elements of Gregory Chanan 's v4 patch - it looks like the v4 patch was against an earlier version of the patch or something, since the removed lines don't exist on trunk. The patch is my attempt at merging current trunk with the additions from the v4 patch. One problem blocking compilation: oldSchema 's update lock is pulled from IndexSchema.getSchemaUpdateLock() , which doesn't exist (that method is on ManagedIndexSchema ) - in my patch, I've cast to oldSchema to ManagedIndexSchema to make that compile, but maybe the intent was to move getSchemaUpdateLock() to IndexSchema ? (That's not part of the v4 patch, maybe git add is needed?) Also, Gregory, when you post patches on JIRA issues, please use the same name for each iteration of your patch, rather than adding "vN", where N increases with each patch version - JIRA will gray out older versions.
        Hide
        Steve Rowe added a comment -

        it looks like the v4 patch was against an earlier version of the patch or something

        Crap, re-reading the comments I see that Gregory Chanan's patch assumes that the patch on SOLR-6180 is applied first, I'll start looking there now - I'm guessing that this is the source of the patch problems I noted above.

        Show
        Steve Rowe added a comment - it looks like the v4 patch was against an earlier version of the patch or something Crap, re-reading the comments I see that Gregory Chanan 's patch assumes that the patch on SOLR-6180 is applied first, I'll start looking there now - I'm guessing that this is the source of the patch problems I noted above.
        Hide
        Gregory Chanan added a comment -

        Crap, re-reading the comments I see that Gregory Chanan's patch assumes that the patch on SOLR-6180 is applied first, I'll start looking there now - I'm guessing that this is the source of the patch problems I noted above.

        Steve Rowe thanks for taking a look. You are correct that SOLR-6180 should be looked at first – sorry for not making that clearer above.

        Also, Gregory, when you post patches on JIRA issues, please use the same name for each iteration of your patch, rather than adding "vN", where N increases with each patch version - JIRA will gray out older versions.

        thanks for letting me know, I'll do that from now on.

        Show
        Gregory Chanan added a comment - Crap, re-reading the comments I see that Gregory Chanan's patch assumes that the patch on SOLR-6180 is applied first, I'll start looking there now - I'm guessing that this is the source of the patch problems I noted above. Steve Rowe thanks for taking a look. You are correct that SOLR-6180 should be looked at first – sorry for not making that clearer above. Also, Gregory, when you post patches on JIRA issues, please use the same name for each iteration of your patch, rather than adding "vN", where N increases with each patch version - JIRA will gray out older versions. thanks for letting me know, I'll do that from now on.
        Hide
        Steve Rowe added a comment -

        Patch with minor modifications from Gregory Chanan's v4 patch:

        1. Modified other schemaless solrconfig.xml update chains (including in the schemaless example) to put Log & DistributedUpdateProcessorFactory before parsing, as in the test config file solrconfig-schemaless.xml
        2. Renamed TestCloudManagedSchemaless->TestCloudSchemaless (schemaless requires managed; shorter = better)
        3. In TestCloudSchemaless, randomized the client to send updates to.

        A couple of points about the previous overview:

        Overview:
        Fixes an NPE issue if there are no dynamic fields and the schema is reloaded

        This is "dynamic copy fields", not "dynamic fields" - I had to enable this fix to run the new test without the other changes, since Solr will NPE otherwise.

        The AddSchemaFieldsUpdateProcessorFactory now implements UpdateRequestProcessorFactory.RunAlways so we can guarantee that the schema additions happen on every core

        This is no longer necessary, since RunAlways only affects processors ordered before the distributed update processor; with Gregory's v4 patch, only the log update processor comes before the distributed update processor, and it already implements RunAlways.


        I'll commit tomorrow if there are no objections.

        Show
        Steve Rowe added a comment - Patch with minor modifications from Gregory Chanan 's v4 patch: Modified other schemaless solrconfig.xml update chains (including in the schemaless example) to put Log & DistributedUpdateProcessorFactory before parsing, as in the test config file solrconfig-schemaless.xml Renamed TestCloudManagedSchemaless->TestCloudSchemaless (schemaless requires managed; shorter = better) In TestCloudSchemaless, randomized the client to send updates to. A couple of points about the previous overview: Overview: Fixes an NPE issue if there are no dynamic fields and the schema is reloaded This is "dynamic copy fields", not "dynamic fields" - I had to enable this fix to run the new test without the other changes, since Solr will NPE otherwise. The AddSchemaFieldsUpdateProcessorFactory now implements UpdateRequestProcessorFactory.RunAlways so we can guarantee that the schema additions happen on every core This is no longer necessary, since RunAlways only affects processors ordered before the distributed update processor; with Gregory's v4 patch, only the log update processor comes before the distributed update processor, and it already implements RunAlways . I'll commit tomorrow if there are no objections.
        Hide
        Steve Rowe added a comment -

        Gregory Chanan, I've committed your patch to trunk and branch_4x with minor modifications (see previous comment).

        I think what's left are:

        Schema API changes return success before all cores are updated; subsequent calls attempting to use new schema may fail

        and

        One small issue I noticed is that there is a race between parsing and schema addition.

        A new issue for this one seems like a good idea.

        Anything else?

        Show
        Steve Rowe added a comment - Gregory Chanan , I've committed your patch to trunk and branch_4x with minor modifications (see previous comment). I think what's left are: Schema API changes return success before all cores are updated; subsequent calls attempting to use new schema may fail and One small issue I noticed is that there is a race between parsing and schema addition. A new issue for this one seems like a good idea. Anything else?
        Hide
        Gregory Chanan added a comment -

        Thanks Use account "steve_rowe" instead! Your changes make sense.

        Schema API changes return success before all cores are updated; subsequent calls attempting to use new schema may fail

        I filed SOLR-6249 for this.

        One small issue I noticed is that there is a race between parsing and schema addition.

        I filed SOLR-6250 for this

        Anything else?

        Nope.

        Show
        Gregory Chanan added a comment - Thanks Use account "steve_rowe" instead ! Your changes make sense. Schema API changes return success before all cores are updated; subsequent calls attempting to use new schema may fail I filed SOLR-6249 for this. One small issue I noticed is that there is a race between parsing and schema addition. I filed SOLR-6250 for this Anything else? Nope.
        Hide
        Steve Rowe added a comment -

        Resolving - remaining issues will be dealt with on the issues Gregory raised.

        Thanks Gregory!

        Show
        Steve Rowe added a comment - Resolving - remaining issues will be dealt with on the issues Gregory raised. Thanks Gregory!

          People

          • Assignee:
            Steve Rowe
            Reporter:
            Gregory Chanan
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development