Solr
  1. Solr
  2. SOLR-5374

Support user configured doc-centric versioning rules

    Details

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

      Description

      The existing optimistic concurrency features of Solr can be very handy for ensuring that you are only updating/replacing the version of the doc you think you are updating/replacing, w/o the risk of someone else adding/removing the doc in the mean time – but I've recently encountered some situations where I really wanted to be able to let the client specify an arbitrary version, on a per document basis, (ie: generated by an external system, or perhaps a timestamp of when a file was last modified) and ensure that the corresponding document update was processed only if the "new" version is greater then the "old" version – w/o needing to check exactly which version is currently in Solr. (ie: If a client wants to index version 101 of a doc, that update should fail if version 102 is already in the index, but succeed if the currently indexed version is 99 – w/o the client needing to ask Solr what the current version)

      The idea Yonik brought up in SOLR-5298 (letting the client specify a _new_version_ that would be used by the existing optimistic concurrency code to control the assignment of the _version_ field for documents) looked like a good direction to go – but after digging into the way _version_ is used internally I realized it requires a uniqueness constraint across all update commands, that would make it impossible to allow multiple independent documents to have the same _version_.

      So instead I've tackled the problem in a different way, using an UpdateProcessor that is configured with user defined field to track a "DocBasedVersion" and uses the RTG logic to figure out if the update is allowed.

      1. SOLR-5374.patch
        11 kB
        Yonik Seeley
      2. SOLR-5374.patch
        70 kB
        Yonik Seeley
      3. SOLR-5374.patch
        55 kB
        Yonik Seeley
      4. SOLR-5374.patch
        36 kB
        Yonik Seeley
      5. SOLR-5374.patch
        39 kB
        Hoss Man
      6. SOLR-5374.patch
        36 kB
        Hoss Man

        Issue Links

          Activity

          Hoss Man created issue -
          Hide
          Hoss Man added a comment -

          Here's an initial patch with some basic docs and tests. There's some nocommits I'm working on regarding configuration validation, and a few TODOs (that i'm less immediately concerned with) related to seeing if I/we can't figure out more efficient ways to do things.

          The basic idea is that at a minimum the user configures a "versionField" which the user can configure to be anything they want, and any time an AddDoc comes in the processor rejects the add if the doc already exists and the value of the new "versionField" isn't greater then the existing value.

          There is also some optional code that can be configured to tell the processor to apply the same rules to deleteById commands using a configured "deleteVersionParam" request param name – if the delete should succeed, then it's actually replaced by an AddDoc that indexes an empty placeholder doc to track the version of the delete so any subsequent "out of order" AddDoc can be rejected.

          Lastly: there's an optional "ignoreOldUpdates" config option, that causes the processor to silently ignore (and return status=200) any updates whose version is too low, instead of returning status=409 (Version Conflict)

          Show
          Hoss Man added a comment - Here's an initial patch with some basic docs and tests. There's some nocommits I'm working on regarding configuration validation, and a few TODOs (that i'm less immediately concerned with) related to seeing if I/we can't figure out more efficient ways to do things. The basic idea is that at a minimum the user configures a "versionField" which the user can configure to be anything they want, and any time an AddDoc comes in the processor rejects the add if the doc already exists and the value of the new "versionField" isn't greater then the existing value. There is also some optional code that can be configured to tell the processor to apply the same rules to deleteById commands using a configured "deleteVersionParam" request param name – if the delete should succeed, then it's actually replaced by an AddDoc that indexes an empty placeholder doc to track the version of the delete so any subsequent "out of order" AddDoc can be rejected. Lastly: there's an optional "ignoreOldUpdates" config option, that causes the processor to silently ignore (and return status=200) any updates whose version is too low, instead of returning status=409 (Version Conflict)
          Hoss Man made changes -
          Field Original Value New Value
          Attachment SOLR-5374.patch [ 12609543 ]
          Hoss Man made changes -
          Link This issue relates to SOLR-5298 [ SOLR-5298 ]
          Hide
          Ramkumar Aiyengar added a comment -

          This feature certainly is helpful as is, but it might not always be practical in a distributed system to have a version ordering across updates as it's likely to involve a single point of coordination if you need one. In such cases, it might just suffice for the system if the versions were just equality comparable rather than having a strict ordering – i.e. update if the previous version equals what I expect, else reject the update. In some sense, if some external coordinator is able to guarantee a version ordering amongst updates, couldn't the same system be able to order the queue of updates to Solr?

          Show
          Ramkumar Aiyengar added a comment - This feature certainly is helpful as is, but it might not always be practical in a distributed system to have a version ordering across updates as it's likely to involve a single point of coordination if you need one. In such cases, it might just suffice for the system if the versions were just equality comparable rather than having a strict ordering – i.e. update if the previous version equals what I expect, else reject the update. In some sense, if some external coordinator is able to guarantee a version ordering amongst updates, couldn't the same system be able to order the queue of updates to Solr?
          Hide
          Hoss Man added a comment -

          it might not always be practical in a distributed system to have a version ordering across updates as it's likely to involve a single point of coordination

          Ramkumar: I'm not sure i understand your comment. The specific use case i'm targeting here is precisely the situation where there is already a an externally generated, per-document, version that we want to use to enforce that only "new" updates are processed. see the issue description:

          I've recently encountered some situations where I really wanted to be able to let the client specify an arbitrary version, on a per document basis, (ie: generated by an external system, or perhaps a timestamp of when a file was last modified) ...

          In such cases, it might just suffice for the system if the versions were just equality comparable rather than having a strict ordering – i.e. update if the previous version equals what I expect, else reject the update

          What you are describing sounds like what is already possible using Solr's existing optimistic concurrency features...

          https://cwiki.apache.org/confluence/display/solr/Updating+Parts+of+Documents#UpdatingPartsofDocuments-OptimisticConcurrency

          I'm trying to address use cases i've seen come up recently where the client app doesn't want to have to check, or keep track of, what's version is in the index (in several cases because they are already keeping track in an independent authoritative data store) they just wants to add/replace a document only if it's "newer" then whatever version is currently in the index.

          Show
          Hoss Man added a comment - it might not always be practical in a distributed system to have a version ordering across updates as it's likely to involve a single point of coordination Ramkumar: I'm not sure i understand your comment. The specific use case i'm targeting here is precisely the situation where there is already a an externally generated, per-document, version that we want to use to enforce that only "new" updates are processed. see the issue description: I've recently encountered some situations where I really wanted to be able to let the client specify an arbitrary version, on a per document basis, (ie: generated by an external system, or perhaps a timestamp of when a file was last modified) ... In such cases, it might just suffice for the system if the versions were just equality comparable rather than having a strict ordering – i.e. update if the previous version equals what I expect, else reject the update What you are describing sounds like what is already possible using Solr's existing optimistic concurrency features... https://cwiki.apache.org/confluence/display/solr/Updating+Parts+of+Documents#UpdatingPartsofDocuments-OptimisticConcurrency I'm trying to address use cases i've seen come up recently where the client app doesn't want to have to check, or keep track of, what's version is in the index (in several cases because they are already keeping track in an independent authoritative data store) they just wants to add/replace a document only if it's "newer" then whatever version is currently in the index.
          Hide
          Ramkumar Aiyengar added a comment -

          My bad, I didn't quite get the Optimistic Concurrency feature, it would indeed do what I was describing. Thanks for the link.

          Show
          Ramkumar Aiyengar added a comment - My bad, I didn't quite get the Optimistic Concurrency feature, it would indeed do what I was describing. Thanks for the link.
          Hide
          Yonik Seeley added a comment -

          It seems like concurrency is not yet handled? Under concurrent updates, the patch won't guarantee the correct ordering.

          Thread 1: update with version=10, check version on doc A, returns 5
          Thread 2: update with version=11, check version on doc A, returns 5
          Thread 2: update completes with version 11
          Thread 1: update completes with version 10
          

          There's going to need to be either some sort of synchronization or optimistic concurrency.

          Also, it looks like the current code assumes it's running on the leader? The realtime-get done is local only, and if you hit the wrong shard with the request, it will look like the doc doesn't exist yet.

          Show
          Yonik Seeley added a comment - It seems like concurrency is not yet handled? Under concurrent updates, the patch won't guarantee the correct ordering. Thread 1: update with version=10, check version on doc A, returns 5 Thread 2: update with version=11, check version on doc A, returns 5 Thread 2: update completes with version 11 Thread 1: update completes with version 10 There's going to need to be either some sort of synchronization or optimistic concurrency. Also, it looks like the current code assumes it's running on the leader? The realtime-get done is local only, and if you hit the wrong shard with the request, it will look like the doc doesn't exist yet.
          Hide
          Hoss Man added a comment -

          It seems like concurrency is not yet handled? Under concurrent updates, the patch won't guarantee the correct ordering.

          You're right ... i hadn't considered that.

          Also, it looks like the current code assumes it's running on the leader? The realtime-get done is local only, ...

          It is?!?! ... I didn't realize that. (but i also hadn't had a chance to add a test for it)

          That must just be because of the convenience method i used correct? Obviously the RTG Component has a way to fetch the document even if you don't hit the correct shard (I hope! for bigger reasons then this patch).

          The only way i can think of to address your concurrency concern is by forcing this logic to run on hte leader (not sure if you have an alternative idea: I'm not following your "or optimistic concurrency." suggestion) in which case if we solve that problem, we should automatically solve the "current code assumes it's running on the leader?" correct?

          Unless i'm missing something, we still don't have an easy generic way to say "run this code only on the leader" – not w/o modifying DistributedUpdateProcessor i don't think – but IIUC the distributed update code first ensures that the update succeeds on the leader before forwarding to the replicas, correct?

          Perhaps we couldtweak the logic of DocBasedVersionConstraintsProcessor so it's configured to run after DistributedUpdateProcessor. On the leader it would use uniqueKey based locking around the existing logic, and throw an error if the constrain wasn't satisfied - preventing the leader from ever forwarding to the replicas. On the replicas it would just be a no-op.

          The "ignoreOldUpdates" would have to be rippped out, but it could easily be refactored into a little convenience processor that could run before DistributedUpdateProcessor so that if enabled it would catch all 409 errors and swallow them. (which could be geenrally re-usable with the existing optimisitc concurrency feature as well if people want to ignore those conflicts as well)

          The only thing i'm not sure about how to deal with if we go this direction is supporting the DeleteUpdateCommand -> AddUpdateCommand logic. Because if that happens on the leader after DistributedUpdateProcessor I don't think it will affect the commands that get forwarded to the replicas. (will it?)

          Show
          Hoss Man added a comment - It seems like concurrency is not yet handled? Under concurrent updates, the patch won't guarantee the correct ordering. You're right ... i hadn't considered that. Also, it looks like the current code assumes it's running on the leader? The realtime-get done is local only, ... It is?!?! ... I didn't realize that. (but i also hadn't had a chance to add a test for it) That must just be because of the convenience method i used correct? Obviously the RTG Component has a way to fetch the document even if you don't hit the correct shard (I hope! for bigger reasons then this patch). The only way i can think of to address your concurrency concern is by forcing this logic to run on hte leader (not sure if you have an alternative idea: I'm not following your "or optimistic concurrency." suggestion) in which case if we solve that problem, we should automatically solve the "current code assumes it's running on the leader?" correct? Unless i'm missing something, we still don't have an easy generic way to say "run this code only on the leader" – not w/o modifying DistributedUpdateProcessor i don't think – but IIUC the distributed update code first ensures that the update succeeds on the leader before forwarding to the replicas, correct? Perhaps we couldtweak the logic of DocBasedVersionConstraintsProcessor so it's configured to run after DistributedUpdateProcessor. On the leader it would use uniqueKey based locking around the existing logic, and throw an error if the constrain wasn't satisfied - preventing the leader from ever forwarding to the replicas. On the replicas it would just be a no-op. The "ignoreOldUpdates" would have to be rippped out, but it could easily be refactored into a little convenience processor that could run before DistributedUpdateProcessor so that if enabled it would catch all 409 errors and swallow them. (which could be geenrally re-usable with the existing optimisitc concurrency feature as well if people want to ignore those conflicts as well) The only thing i'm not sure about how to deal with if we go this direction is supporting the DeleteUpdateCommand -> AddUpdateCommand logic. Because if that happens on the leader after DistributedUpdateProcessor I don't think it will affect the commands that get forwarded to the replicas. (will it?)
          Hide
          Hoss Man added a comment -

          Updated patch - no new functionality, just an additional test method demonstrating the concurrency issue (on a single node) that yonik brought up.

          Show
          Hoss Man added a comment - Updated patch - no new functionality, just an additional test method demonstrating the concurrency issue (on a single node) that yonik brought up.
          Hoss Man made changes -
          Attachment SOLR-5374.patch [ 12610192 ]
          Hide
          Yonik Seeley added a comment - - edited

          Hmmm, testConcurrentAdds fails even if I change the executor size to 1 thread... not sure why at this point.

          edit: found it - a test bug where sometimes the winner was chosen for deletion when it wasn't supposed to be. Test now passes with single-thread executor and fails with more threads. Next up: optimistic concurrency to fix the concurrency issues.

          Oh, another random note: we must have this processor run before the distributed update processor that handles internal solr versions. This is because that processor could end up dropping a winning update (i.e. a doc that has a higher external version but was assigned a lower internal version).

          Show
          Yonik Seeley added a comment - - edited Hmmm, testConcurrentAdds fails even if I change the executor size to 1 thread... not sure why at this point. edit: found it - a test bug where sometimes the winner was chosen for deletion when it wasn't supposed to be. Test now passes with single-thread executor and fails with more threads. Next up: optimistic concurrency to fix the concurrency issues. Oh, another random note: we must have this processor run before the distributed update processor that handles internal solr versions. This is because that processor could end up dropping a winning update (i.e. a doc that has a higher external version but was assigned a lower internal version).
          Hide
          Yonik Seeley added a comment -

          Ideally, this code would run on the leader for the shard.
          I've opened SOLR-5395 as one step to allow that.

          Show
          Yonik Seeley added a comment - Ideally, this code would run on the leader for the shard. I've opened SOLR-5395 as one step to allow that.
          Yonik Seeley made changes -
          Link This issue depends upon SOLR-5395 [ SOLR-5395 ]
          Hide
          Yonik Seeley added a comment -

          Here's a patch that includes implementation of multi-threaded fixes using optimistic concurrency.

          Show
          Yonik Seeley added a comment - Here's a patch that includes implementation of multi-threaded fixes using optimistic concurrency.
          Yonik Seeley made changes -
          Attachment SOLR-5374.patch [ 12610708 ]
          Hide
          Yonik Seeley added a comment -

          hmmm, in SolrCloud mode, somewhere in the mix del_version is being dropped. Not sure where yet...

          Show
          Yonik Seeley added a comment - hmmm, in SolrCloud mode, somewhere in the mix del_version is being dropped. Not sure where yet...
          Yonik Seeley made changes -
          Link This issue depends upon SOLR-5406 [ SOLR-5406 ]
          Hide
          Yonik Seeley added a comment -

          Linking to SOLR-5406, which hopefully is the only issue stopping this from fully working.

          Show
          Yonik Seeley added a comment - Linking to SOLR-5406 , which hopefully is the only issue stopping this from fully working.
          Hide
          Yonik Seeley added a comment -

          OK, it seems like things are working in distributed mode now... a few more cleanups and it will be ready to commit.

          Show
          Yonik Seeley added a comment - OK, it seems like things are working in distributed mode now... a few more cleanups and it will be ready to commit.
          Yonik Seeley made changes -
          Attachment SOLR-5374.patch [ 12611393 ]
          Hide
          Yonik Seeley added a comment -

          OK, here's the final patch with everything cleaned up, and with a new stress test and optimizations to use the fieldcache/docvalues when possible instead of always going to stored fields.

          Show
          Yonik Seeley added a comment - OK, here's the final patch with everything cleaned up, and with a new stress test and optimizations to use the fieldcache/docvalues when possible instead of always going to stored fields.
          Yonik Seeley made changes -
          Attachment SOLR-5374.patch [ 12611449 ]
          Hide
          ASF subversion and git services added a comment -

          Commit 1537587 from Yonik Seeley in branch 'dev/trunk'
          [ https://svn.apache.org/r1537587 ]

          SOLR-5374: user version update processor

          Show
          ASF subversion and git services added a comment - Commit 1537587 from Yonik Seeley in branch 'dev/trunk' [ https://svn.apache.org/r1537587 ] SOLR-5374 : user version update processor
          Hide
          ASF subversion and git services added a comment -

          Commit 1537597 from Yonik Seeley in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1537597 ]

          SOLR-5374: user version update processor

          Show
          ASF subversion and git services added a comment - Commit 1537597 from Yonik Seeley in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1537597 ] SOLR-5374 : user version update processor
          Yonik Seeley made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 4.6 [ 12325000 ]
          Fix Version/s 5.0 [ 12321664 ]
          Resolution Fixed [ 1 ]
          Hide
          ASF subversion and git services added a comment -

          Commit 1537704 from Yonik Seeley in branch 'dev/trunk'
          [ https://svn.apache.org/r1537704 ]

          SOLR-5374: fix unnamed thread pool

          Show
          ASF subversion and git services added a comment - Commit 1537704 from Yonik Seeley in branch 'dev/trunk' [ https://svn.apache.org/r1537704 ] SOLR-5374 : fix unnamed thread pool
          Hide
          ASF subversion and git services added a comment -

          Commit 1537706 from Yonik Seeley in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1537706 ]

          SOLR-5374: fix unnamed thread pool

          Show
          ASF subversion and git services added a comment - Commit 1537706 from Yonik Seeley in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1537706 ] SOLR-5374 : fix unnamed thread pool
          Hide
          Yonik Seeley added a comment -

          There was a bug where we missed calling "return" after delegating to the next in the chain. This meant that (among other things) the code that was only meant to run on the leader was run on every node the update touched. In the default "ignore old updates" mode, this didn't cause any failures since updates are idempotent, but it does cause failures when a 409 (Conflict) is thrown instead.

          This patch fixes the bug and adds test coverage.

          Show
          Yonik Seeley added a comment - There was a bug where we missed calling "return" after delegating to the next in the chain. This meant that (among other things) the code that was only meant to run on the leader was run on every node the update touched. In the default "ignore old updates" mode, this didn't cause any failures since updates are idempotent, but it does cause failures when a 409 (Conflict) is thrown instead. This patch fixes the bug and adds test coverage.
          Yonik Seeley made changes -
          Attachment SOLR-5374.patch [ 12612994 ]
          Hide
          ASF subversion and git services added a comment -

          Commit 1540336 from Yonik Seeley in branch 'dev/trunk'
          [ https://svn.apache.org/r1540336 ]

          SOLR-5374: missing returns in user versioning processor

          Show
          ASF subversion and git services added a comment - Commit 1540336 from Yonik Seeley in branch 'dev/trunk' [ https://svn.apache.org/r1540336 ] SOLR-5374 : missing returns in user versioning processor
          Hide
          ASF subversion and git services added a comment -

          Commit 1540341 from Yonik Seeley in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1540341 ]

          SOLR-5374: missing returns in user versioning processor

          Show
          ASF subversion and git services added a comment - Commit 1540341 from Yonik Seeley in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1540341 ] SOLR-5374 : missing returns in user versioning processor
          Hide
          Anshum Gupta added a comment -

          Just a thought, should we change the commented logging to log.debug?
          I'm assuming that's the intention behind leaving it in there.

          Show
          Anshum Gupta added a comment - Just a thought, should we change the commented logging to log.debug? I'm assuming that's the intention behind leaving it in there.
          Hide
          Yonik Seeley added a comment -

          should we change the commented logging to log.debug?

          I only left them there (commented out) in case I needed to try and debug again in the short term. They are not of the quality one would want for long term. I'd rather they be deleted than changed to logs.

          Show
          Yonik Seeley added a comment - should we change the commented logging to log.debug? I only left them there (commented out) in case I needed to try and debug again in the short term. They are not of the quality one would want for long term. I'd rather they be deleted than changed to logs.
          Hide
          Linbin Chen added a comment -

          good feature. extremely useful

          Show
          Linbin Chen added a comment - good feature. extremely useful
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          9d 21h 50m 1 Yonik Seeley 31/Oct/13 20:03

            People

            • Assignee:
              Hoss Man
              Reporter:
              Hoss Man
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development