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

DCP protocol change to include high "visible" seqno

    XMLWordPrintable

Details

    • Triaged
    • Unknown
    • KV-Engine Mad-Hatter GA

    Description

      GetAllVBSeqnos need to be sure that the seqno returned to the caller is not a prepare/abort.

      A DCP protocol update is required (an extension of the sync-write DCP protocol changes already made in mad-hatter). The reason for the protocol change is to solve an issue in GetAllVBSeqnos for replica vbuckets.

      When we return the 'high-seqno' of a replica, we use the replica's current snapshot.end. The value exposed in this case can, if the replica is receiving a disk snapshot suffer the same issue described in MB-36948, in that it could be a prepare/abort. If the caller of GetAllVBSeqnos does not opt into sync-replication, then it expects to see the snapshot.end on DCP, however it won't (the view-engine will do exactly that and likely block rebalance, or just hang).

      To resolve the issue the DCP snapshot marker command needs to transmit the extra seqno (which won't be a prepare/abort) from active to replica and then GetAllVBSeqnos can use that value for the replica seqno if the caller hasn't negotiated sync-replication.

      This MB covers the change to the DCP protocol.

      Attachments

        Issue Links

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

          Activity

            lynn.straus Lynn Straus added a comment -

            converted to type "bug" for visibility and added "approved for mad hatter" label per review in Nov 21 MH ops meeting

            lynn.straus Lynn Straus added a comment - converted to type "bug" for visibility and added "approved for mad hatter" label per review in Nov 21 MH ops meeting
            jwalker Jim Walker added a comment - - edited

            This change is good to go, one new piece of information (after more thinking on MB-36948).

            At the moment in mad-hatter the V2 snapshot packet is only sent for certain conditions, disk snapshots only, when we send a memory snapshot the V1 format is still used. For MB-36948 this will have to change because to solve the issue with GetAllVBSeqnos and replica VB, the kv to kv DCP stream needs to always be sending the new 'visible-seqno' value for all snapshots (memory and disk, and only when sync-replication has been enabled).

            Pro: issue MB-36948 can be addressed
            Con: The DCP protocol increases in size

            More work could be done so that we effectively have V1, V2 and V3 formats, and only increase the more common memory snapshot by 8 bytes (or is it time to use leb128)

            • V1 - pre mad-hatter format (flags, start + end)
            • V2 - sends (flags, start + end) + max-visible-seqno and is used for memory snapshots
            • V3 - sends V2 + highCompletedSeqno and is used for disk snapshots
            jwalker Jim Walker added a comment - - edited This change is good to go, one new piece of information (after more thinking on MB-36948 ). At the moment in mad-hatter the V2 snapshot packet is only sent for certain conditions, disk snapshots only, when we send a memory snapshot the V1 format is still used. For MB-36948 this will have to change because to solve the issue with GetAllVBSeqnos and replica VB, the kv to kv DCP stream needs to always be sending the new 'visible-seqno' value for all snapshots (memory and disk, and only when sync-replication has been enabled). Pro: issue MB-36948 can be addressed Con: The DCP protocol increases in size More work could be done so that we effectively have V1, V2 and V3 formats, and only increase the more common memory snapshot by 8 bytes (or is it time to use leb128) V1 - pre mad-hatter format (flags, start + end) V2 - sends (flags, start + end) + max-visible-seqno and is used for memory snapshots V3 - sends V2 + highCompletedSeqno and is used for disk snapshots
            owend Daniel Owen added a comment -

            Pushing target date out until the end of Tuesday (26th).

            owend Daniel Owen added a comment - Pushing target date out until the end of Tuesday (26th).
            jwalker Jim Walker added a comment -

            An updated version of the code for this MB has been pushed to review which aims to address all the needs of MB-36948, this commit adjusts the mad-hatter snapshot protocol changes to be more flexible so we can more easily encode the 3 'versions' as identified earlier.

            This change still needs cv/review and probably some technical discussion...

            jwalker Jim Walker added a comment - An updated version of the code for this MB has been pushed to review which aims to address all the needs of MB-36948 , this commit adjusts the mad-hatter snapshot protocol changes to be more flexible so we can more easily encode the 3 'versions' as identified earlier. http://review.couchbase.org/#/c/118387/ This change still needs cv/review and probably some technical discussion...
            jwalker Jim Walker added a comment -

            An updated patch is in review, CV raises discussion about keeping the flexibility of the protocol change (use of version field) and simplicity.

            A) Keep patch as is (3 encodings - legacy and 2 new encodings)
            B) Reduce complexity - use 2 encodings, but always send the hcs as 0 for in-memory snapshots

            Note: The latest uploaded patch identified a dcp buffer acknowledgment issue which has been addressed in the change - the current mad-hatter code has the wrong size in the buffer calculation (see response.h/response.cc changes), which could lead to an underflow, i.e. the consumer may ack more than the producer has logged (which results in a warning logged).

            jwalker Jim Walker added a comment - An updated patch is in review, CV raises discussion about keeping the flexibility of the protocol change (use of version field) and simplicity. A) Keep patch as is (3 encodings - legacy and 2 new encodings) B) Reduce complexity - use 2 encodings, but always send the hcs as 0 for in-memory snapshots Note: The latest uploaded patch identified a dcp buffer acknowledgment issue which has been addressed in the change - the current mad-hatter code has the wrong size in the buffer calculation (see response.h/response.cc changes), which could lead to an underflow, i.e. the consumer may ack more than the producer has logged (which results in a warning logged).

            Build couchbase-server-6.5.0-4902 contains kv_engine commit 8d8ad4c with commit message:
            MB-37013: Update DcpSnapShotMarker V2 to allow for an extra seqno

            build-team Couchbase Build Team added a comment - Build couchbase-server-6.5.0-4902 contains kv_engine commit 8d8ad4c with commit message: MB-37013 : Update DcpSnapShotMarker V2 to allow for an extra seqno

            Build couchbase-server-7.0.0-1100 contains kv_engine commit 8d8ad4c with commit message:
            MB-37013: Update DcpSnapShotMarker V2 to allow for an extra seqno

            build-team Couchbase Build Team added a comment - Build couchbase-server-7.0.0-1100 contains kv_engine commit 8d8ad4c with commit message: MB-37013 : Update DcpSnapShotMarker V2 to allow for an extra seqno

            Daniel Owen and Jim Walker - please help with steps to do functional verification.

            ritam.sharma Ritam Sharma added a comment - Daniel Owen and Jim Walker - please help with steps to do functional verification.

            People

              owend Daniel Owen
              owend Daniel Owen
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty