Uploaded image for project: 'Couchbase Gateway'
  1. Couchbase Gateway
  2. CBG-551

Avoid storing _removed:true revision bodies in the revision cache

    XMLWordPrintable

Details

    • Improvement
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • None
    • 3.0
    • SyncGateway
    • Security Level: Public
    • None
    • CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40, CBG Sprint 62, CBG Sprint 63
    • 5

    Description

      Normally _removed:true is calculated post-retrieval from the revision cache, depending on whether the user is authenticated for the revision. The exception is revision cache placeholder entries where Sync Gateway no longer has the document body, but knows from the document's channel set that the revision represented a channel removal.

      This approach has two problems:
      1. For documents in multiple channels, it's not necessarily the case that a given revision in this scenario is a removal from a channel the requesting user has access to.
      2. Storing _removed:true in the body diverges from the Mercury design approach of not storing special properties in the document in the revision cache.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          adamf Adam Fraser created issue -
          adamf Adam Fraser made changes -
          Field Original Value New Value
          Fix Version/s Hydrogen [ 16179 ]
          Fix Version/s Mercury [ 15421 ]
          adamf Adam Fraser made changes -
          Priority Minor [ 4 ] Major [ 3 ]
          adamf Adam Fraser made changes -
          Priority Major [ 3 ] Critical [ 2 ]
          adamf Adam Fraser made changes -
          Assignee The One [ the one ] Adam Fraser [ adamf ]
          adamf Adam Fraser added a comment -

          Assigning to myself to write a unit test that reproduces the scenario, and implementation details on what needs to be changed.

          adamf Adam Fraser added a comment - Assigning to myself to write a unit test that reproduces the scenario, and implementation details on what needs to be changed.
          adamf Adam Fraser made changes -
          Sprint CBG Sprint 37 [ 943 ]
          adamf Adam Fraser made changes -
          Rank Ranked lower
          ben.brooks Ben Brooks made changes -
          Sprint CBG Sprint 37 [ 943 ] CBG Sprint 37, CBG Sprint 38 [ 943, 948 ]
          adamf Adam Fraser made changes -
          Sprint CBG Sprint 37, CBG Sprint 38 [ 943, 948 ] CBG Sprint 37, CBG Sprint 38, CBG Sprint 39 [ 943, 948, 960 ]
          ben.brooks Ben Brooks made changes -
          Sprint CBG Sprint 37, CBG Sprint 38, CBG Sprint 39 [ 943, 948, 960 ] CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40 [ 943, 948, 960, 969 ]
          adamf Adam Fraser made changes -
          Sprint CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40 [ 943, 948, 960, 969 ] CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40, CBG Sprint 41 [ 943, 948, 960, 969, 980 ]
          adamf Adam Fraser made changes -
          Sprint CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40, CBG Sprint 41 [ 943, 948, 960, 969, 980 ] CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40 [ 943, 948, 960, 969 ]
          adamf Adam Fraser made changes -
          Rank Ranked higher
          adamf Adam Fraser made changes -
          Rank Ranked higher
          daniel.petersen Daniel Petersen made changes -
          Rank Ranked higher
          adamf Adam Fraser made changes -
          Fix Version/s Lithium [ 16180 ]
          Fix Version/s Hydrogen [ 16179 ]
          adamf Adam Fraser made changes -
          Story Points 5 3
          adamf Adam Fraser added a comment -

          The first item is already covered by the recent changes in CBG-1042.  CBG-1042 still depends on setting a "removed:true" body in the rev cache, as it uses this to identify removal entries as part of the calculation of whether the user had access to the revision via a different channel.

          It would still be preferable to avoid using the body for this.  Need to review whether we can store a nil/empty body in the revision cache and use a new metadata property in the revision cache entry ('isRemoved') to identify removals. 

          adamf Adam Fraser added a comment - The first item is already covered by the recent changes in CBG-1042.  CBG-1042 still depends on setting a "removed:true" body in the rev cache, as it uses this to identify removal entries as part of the calculation of whether the user had access to the revision via a different channel. It would still be preferable to avoid using the body for this.  Need to review whether we can store a nil/empty body in the revision cache and use a new metadata property in the revision cache entry ('isRemoved') to identify removals. 
          adamf Adam Fraser made changes -
          Assignee Adam Fraser [ adamf ] Sarath Kumar Sivan [ sarath.kumarsivan ]
          adamf Adam Fraser made changes -
          Story Points 3 5
          adamf Adam Fraser made changes -
          Sprint CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40 [ 943, 948, 960, 969 ] CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40, CBG Sprint 62 [ 943, 948, 960, 969, 1324 ]
          adamf Adam Fraser made changes -
          Rank Ranked lower
          sarath.kumarsivan Sarath Kumar Sivan (Inactive) made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Automated transition triggered when Sarath Kumar Sivan created pull request #4873 in GitHub -
          Status In Progress [ 3 ] In Review [ 10107 ]
          adamf Adam Fraser made changes -
          Sprint CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40, CBG Sprint 62 [ 943, 948, 960, 969, 1324 ] CBG Sprint 37, CBG Sprint 38, CBG Sprint 39, CBG Sprint 40, CBG Sprint 62, CBG Sprint 63 [ 943, 948, 960, 969, 1324, 1335 ]
          Automated transition triggered when Adam Fraser merged pull request #4873 in GitHub -
          Resolution Fixed [ 1 ]
          Status In Review [ 10107 ] Resolved [ 5 ]

          Build sync_gateway-3.0.0-76 contains sync_gateway commit 2d75ce7 with commit message:
          CBG-551 Avoid storing _removed:true revision bodies in the revision cache (#4873)

          build-team Couchbase Build Team added a comment - Build sync_gateway-3.0.0-76 contains sync_gateway commit 2d75ce7 with commit message: CBG-551 Avoid storing _removed:true revision bodies in the revision cache (#4873)
          sarath.kumarsivan Sarath Kumar Sivan (Inactive) made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

          People

            sarath.kumarsivan Sarath Kumar Sivan (Inactive)
            adamf Adam Fraser
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty