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

Skip-deleted-docs optimization never enabled in pull replication

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.7.0
    • Component/s: None
    • Security Level: Public
    • Labels:
      None
    • Sprint:
      CBG Sprint 34
    • Story Points:
      1

      Description

      On the first pull into an empty database, LiteCore should tell SG not to send any deleted documents (tombstones), by setting the "activeOnly" property in the "subChanges" request. Unfortunately this is not hooked up.

      The result is reduced replication performance on the first pull, and a larger-than-necessary local database, if there are a lot of tombstones on the server. No functional problems.

      Here's what I found:

      • The Puller sets "activeOnly" if its member _skipDeleted is true. (Puller.cc:71)
      • _skipDeleted is set if the replicator option kC4ReplicatorOptionSkipDeleted is set, or if the _setSkipDeleted() method is called.
      • There is no code that sets the kC4ReplicatorOptionSkipDeleted option (in LiteCore or CBL-iOS; I would assume not on Android or .NET either since we have no public API to set this.)
      • There are no calls to _setSkipDeleted() in LiteCore.

      It looks as though the Replicator class was supposed to call _setSkipDeleted() if the database is empty and there is no checkpoint, but that logic was either deleted by accident or never written.

        Attachments

          Issue Links

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

            Activity

            Hide
            jimb Jim Borden added a comment -

            No, we don't have any developer side testing that interacts (at least not without manually doing things) with a live Sync Gateway instance.  I think I can retrace my steps and verify manually if you want though.

            Show
            jimb Jim Borden added a comment - No, we don't have any developer side testing that interacts (at least not without manually doing things) with a live Sync Gateway instance.  I think I can retrace my steps and verify manually if you want though.
            Hide
            adamf Adam Fraser added a comment -

            Jim Borden That would be great, but if you've got any ideas about how we can get QE coverage of this, that sounds like it would be a good idea.

            Show
            adamf Adam Fraser added a comment - Jim Borden That would be great, but if you've got any ideas about how we can get QE coverage of this, that sounds like it would be a good idea.
            Hide
            jimb Jim Borden added a comment -

            I think the following functional test should suffice:

            1. Add some documents to SG
            2. Delete them all
            3. Run a pull replication to CBL
            4. Observe that the last sequence in the local database is unchanged since no documents are pulled
            Show
            jimb Jim Borden added a comment - I think the following functional test should suffice: Add some documents to SG Delete them all Run a pull replication to CBL Observe that the last sequence in the local database is unchanged since no documents are pulled
            Hide
            jimb Jim Borden added a comment -

            I ran a test in my sandbox program and monitored with wireshark.  The above conversation is the same up to the MSG#1 from SG.  Now instead of a change, it contains null as I believe it should.

            Show
            jimb Jim Borden added a comment - I ran a test in my sandbox program and monitored with wireshark.  The above conversation is the same up to the MSG#1 from SG.  Now instead of a change, it contains null as I believe it should.
            Hide
            adamf Adam Fraser added a comment -

            Jim Borden Thanks for verifying.

            Sridevi Saragadam It would be useful to have a QE test like the one Jim describes above - do you want to file a tracking ticket for that?

            Show
            adamf Adam Fraser added a comment - Jim Borden Thanks for verifying. Sridevi Saragadam It would be useful to have a QE test like the one Jim describes above - do you want to file a tracking ticket for that?

              People

              Assignee:
              adamf Adam Fraser
              Reporter:
              jens Jens Alfke
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Gerrit Reviews

                  There are no open Gerrit changes

                    PagerDuty