Uploaded image for project: 'Couchbase Server'
  1. Couchbase Server
  2. MB-37330

Checkpoint expelling can skip expellable items

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 6.5.0
    • 6.5.1
    • couchbase-bucket
    • None
    • Triaged
    • No

    Description

      Summary

      The checkpoint expelling code added in Mad-Hatter can fail to expel an item from a Checkpoint if there's only 1 candidate item to expel.

      This can result in increased memory usage (see MB-37294), which is more significant for large Documents / small memory quotas.

      (Separated out from MB-37294.)

      Steps to Reproduce
      Difficult to precisely trigger this scenario in full-stack tests, however you want to attempt to end up with a Checkpoint as follows, and then have expelling be attempted:

      CheckpointManager[0x105dfa740] with numItems:3 checkpoints:1
          Checkpoint[0x105f3b000] with id:1 seqno:{1001,1002} snap:{0,1002, visible:1002} state:CHECKPOINT_OPEN numCursors:1 type:Memory hcs:-- items:[
      	{1001,empty,cid:0x1:empty,110,[m]}
      	{1001,checkpoint_start,cid:0x1:checkpoint_start,121,[m]}
      	{1001,mutation,cid:0x0:key0,109,}
      	{1002,mutation,cid:0x0:key1,109,}
      ]
          cursors:[
              persistence: CheckpointCursor[0x105c5d998] with name:persistence currentCkpt:{id:1 state:CHECKPOINT_OPEN} currentPos:1002
          ]
      

      • Two adjacent mutations (at seqnos 1001 and 1002).
      • The the first mutation shares its seqno with the remainder of the items in the checkpoint (here with meta-items checkpoint_start & empty).

      Note that checkpoint state can be examined via:

      cbstats localhost:11210 -u Administrator -p <PASSWORD> raw "_checkpoint-dump <VBID>
      

      Any set regular workload under DGM (e.g. pillowfight) should trigger this.

      Expected Results
      Such checkpoints should allow all but the last item (1002) to be expelled:

      CheckpointManager[0x105efb740] with numItems:3 checkpoints:1
          Checkpoint[0x10803c000] with id:1 seqno:{1002,1002} snap:{0,1002, visible:1002} state:CHECKPOINT_OPEN numCursors:1 type:Memory hcs:-- items:[
      	{1001,empty,cid:0x1:empty,110,[m]}
      	{1002,mutation,cid:0x0:key1,109,}
      ]
          cursors:[
              persistence: CheckpointCursor[0x105c5e998] with name:persistence currentCkpt:{id:1 state:CHECKPOINT_OPEN} currentSeq:1002 distance:1
          ]
      

      Actual Results
      No items are expelled.

      Attachments

        Issue Links

          For Gerrit Dashboard: MB-37330
          # Subject Branch Project Status CR V

          Activity

            drigby Dave Rigby added a comment -

            The issue is the logic in the expel code which attempts to ensure that items with the same seqno are not partially expelled - i.e. we should expel either all or none of items sharing the same seqno.

            This is done by adjusting the chosen expel cursor backwards (initially set to the earliest cursor) if it points to an invalid position - from the code:

                     /*
                      * Walk backwards over the checkpoint if not yet reached the dummy item,
                      * and pointing to an item that either:
                      * 1. has a seqno equal to the checkpoint's high seqno, or
                      * 2. has a previous entry with the same seqno, or
                      * 3. is pointing to a metadata item.
                      */
                     while ((iterator != oldestCheckpoint->begin()) &&
                            (((*iterator)->getBySeqno() ==
                              int64_t(oldestCheckpoint->getHighSeqno())) ||
                             ((*std::prev(iterator))->getBySeqno() ==
                              (*iterator)->getBySeqno()) ||
                             ((*iterator)->isCheckPointMetaItem()))) {
                         --iterator;
                     }
            

            The intent here is correct, however the specifics are slightly wrong.

            Consider the previous example - note the oldest (only) cursor is pointing at mutation:1002:

            empty  checkpoint_start  mutation  mutation
            1001   1001              1001      1002
                                               ^
                                               Cursor
            

            The while loop is executed and the first clause is true - Cursor is pointing at the high seqno (we cannot expel the last item in a checkpoint, must always leave at least one item). The cursor is correctly decremented:

            empty  checkpoint_start  mutation  mutation
            1001   1001              1001      1002
                                     ^
                                     Cursor
            

            This should be where the cursor stops - it is safe to expel everything from the Cursor backwards (inclusive), given that wouldn't leave some items with the same seqno expelled.

            However, clause (2) checks if the previous item (checkpoint_start:1001) shares the same seqno as the current item (mutation:1001). This is true, and the cursor is decremented.

            The same thing happens again for the next position (empty:1001 and checkpoint_start:1001) and hence the cursor ends up at the start of the checkpoint and nothing is expelled:

            empty  checkpoint_start  mutation  mutation
            1001   1001              1001      1002
            ^
            Cursor
            

            What should have happened is the seqno of the current item should be compared with the next item (mutation:1001 and mutation:1002), which is false and hence the iterator should have stopped when pointing at mutation:1001.

            drigby Dave Rigby added a comment - The issue is the logic in the expel code which attempts to ensure that items with the same seqno are not partially expelled - i.e. we should expel either all or none of items sharing the same seqno. This is done by adjusting the chosen expel cursor backwards (initially set to the earliest cursor) if it points to an invalid position - from the code: /* * Walk backwards over the checkpoint if not yet reached the dummy item, * and pointing to an item that either: * 1. has a seqno equal to the checkpoint's high seqno, or * 2. has a previous entry with the same seqno, or * 3. is pointing to a metadata item. */ while ((iterator != oldestCheckpoint->begin()) && (((*iterator)->getBySeqno() == int64_t(oldestCheckpoint->getHighSeqno())) || ((*std::prev(iterator))->getBySeqno() == (*iterator)->getBySeqno()) || ((*iterator)->isCheckPointMetaItem()))) { --iterator; } The intent here is correct, however the specifics are slightly wrong. Consider the previous example - note the oldest (only) cursor is pointing at mutation:1002: empty checkpoint_start mutation mutation 1001 1001 1001 1002 ^ Cursor The while loop is executed and the first clause is true - Cursor is pointing at the high seqno (we cannot expel the last item in a checkpoint, must always leave at least one item). The cursor is correctly decremented: empty checkpoint_start mutation mutation 1001 1001 1001 1002 ^ Cursor This should be where the cursor stops - it is safe to expel everything from the Cursor backwards (inclusive), given that wouldn't leave some items with the same seqno expelled. However, clause (2) checks if the previous item (checkpoint_start:1001) shares the same seqno as the current item (mutation:1001). This is true , and the cursor is decremented. The same thing happens again for the next position (empty:1001 and checkpoint_start:1001) and hence the cursor ends up at the start of the checkpoint and nothing is expelled: empty checkpoint_start mutation mutation 1001 1001 1001 1002 ^ Cursor What should have happened is the seqno of the current item should be compared with the next item (mutation:1001 and mutation:1002), which is false and hence the iterator should have stopped when pointing at mutation:1001.

            Build couchbase-server-6.5.1-6023 contains kv_engine commit 5417af9 with commit message:
            MB-37330: Correct expelling logic for items with same seqno

            build-team Couchbase Build Team added a comment - Build couchbase-server-6.5.1-6023 contains kv_engine commit 5417af9 with commit message: MB-37330 : Correct expelling logic for items with same seqno

            Build couchbase-server-7.0.0-1162 contains kv_engine commit 5417af9 with commit message:
            MB-37330: Correct expelling logic for items with same seqno

            build-team Couchbase Build Team added a comment - Build couchbase-server-7.0.0-1162 contains kv_engine commit 5417af9 with commit message: MB-37330 : Correct expelling logic for items with same seqno
            ritam.sharma Ritam Sharma added a comment -

            Sumedh Basarkod - Can you please help with this validation. Ashwin Govindarajulu - Can you help Sumedh with validation.

            ritam.sharma Ritam Sharma added a comment - Sumedh Basarkod - Can you please help with this validation. Ashwin Govindarajulu - Can you help Sumedh with validation.

            Closing the issue, validated by running the corresponding unit test cases(expelCursorPointingToLastItem) and they pass.

            sumedh.basarkod Sumedh Basarkod added a comment - Closing the issue, validated by running the corresponding unit test cases(expelCursorPointingToLastItem) and they pass.

            Build couchbase-server-6.6.0-7519 contains kv_engine commit 5417af9 with commit message:
            MB-37330: Correct expelling logic for items with same seqno

            build-team Couchbase Build Team added a comment - Build couchbase-server-6.6.0-7519 contains kv_engine commit 5417af9 with commit message: MB-37330 : Correct expelling logic for items with same seqno

            People

              drigby Dave Rigby
              drigby Dave Rigby
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty