[fix #192] fix unified sorter (#194)

* fix unified sorter Signed-off-by: zeminzhou <[email protected]> * delete log Signed-off-by: zeminzhou <[email protected]> * fix ut Signed-off-by: zeminzhou <[email protected]> * fix comment Signed-off-by: zeminzhou <[email protected]> * fix lint Signed-off-by: zeminzhou <[email protected]> * fix lint Signed-off-by: zeminzhou <[email protected]>
tikv · Aug 4, 2022 · 86810be · 86810be
1 parent 769754e
commit 86810be
Show file tree

Hide file tree

Showing 11 changed files with 282 additions and 22 deletions.
diff --git a/cdc/cdc/model/kv.go b/cdc/cdc/model/kv.go
@@ -87,11 +87,15 @@ type RawKVEntry struct {
 	// Additional debug info
 	RegionID  uint64 `msg:"region_id"`
 	KeySpanID uint64 `msg:"keyspan_id"`
+	// For providing additional sequence number.
+	// To keep `RawKVEntries` from the same region in order during unstable sorting of `sorter`.
+	// The sequence number is generated by auto-increment in `puller` node of `processor.pipeline`.
+	Sequence uint64 `msg:"sequence"`
 }
 
 func (v *RawKVEntry) String() string {
-	return fmt.Sprintf("OpType: %v, Key: %s, Value: %s, StartTs: %d, CRTs: %d, RegionID: %d",
-		v.OpType, string(v.Key), string(v.Value), v.StartTs, v.CRTs, v.RegionID)
+	return fmt.Sprintf("OpType: %v, Key: %s, Value: %s, StartTs: %d, CRTs: %d, RegionID: %d, KeySpanID: %d, Sequence: %d",
+		v.OpType, string(v.Key), string(v.Value), v.StartTs, v.CRTs, v.RegionID, v.KeySpanID, v.Sequence)
 }
 
 // ApproximateDataSize calculate the approximate size of protobuf binary

diff --git a/cdc/cdc/model/kv_gen.go b/cdc/cdc/model/kv_gen.go
diff --git a/cdc/cdc/model/kv_gen_test.go b/cdc/cdc/model/kv_gen_test.go
diff --git a/cdc/cdc/model/kv_test.go b/cdc/cdc/model/kv_test.go
@@ -55,6 +55,6 @@ func TestRawKVEntry(t *testing.T) {
 		Value:   []byte("345"),
 	}
 
-	require.Equal(t, "OpType: 1, Key: 123, Value: 345, StartTs: 100, CRTs: 101, RegionID: 0", raw.String())
+	require.Equal(t, "OpType: 1, Key: 123, Value: 345, StartTs: 100, CRTs: 101, RegionID: 0, KeySpanID: 0, Sequence: 0", raw.String())
 	require.Equal(t, int64(6), raw.ApproximateDataSize())
 }
diff --git a/cdc/cdc/model/mounter.go b/cdc/cdc/model/mounter.go
@@ -60,6 +60,11 @@ func (e *PolymorphicEvent) IsResolved() bool {
 	return e.RawKV.OpType == OpTypeResolved
 }
 
+// Sequence returns the sequence of the event.
+func (e *PolymorphicEvent) Sequence() uint64 {
+	return e.RawKV.Sequence
+}
+
 // ComparePolymorphicEvents compares two events by CRTs, Resolved order.
 // It returns true if and only if i should precede j.
 func ComparePolymorphicEvents(i, j *PolymorphicEvent) bool {
@@ -69,6 +74,8 @@ func ComparePolymorphicEvents(i, j *PolymorphicEvent) bool {
 		} else if j.IsResolved() {
 			return true
 		}
+
+		return i.Sequence() < j.Sequence()
 	}
 	return i.CRTs < j.CRTs
 }

diff --git a/cdc/cdc/model/mounter_test.go b/cdc/cdc/model/mounter_test.go
@@ -72,3 +72,121 @@ func TestPolymorphicEventPrepare(t *testing.T) {
 	err := polyEvent.WaitPrepare(cctx)
 	require.Equal(t, context.Canceled, err)
 }
+
+func TestComparePolymorphicEvents(t *testing.T) {
+	testCases := []struct {
+		i        *PolymorphicEvent
+		j        *PolymorphicEvent
+		expected bool
+	}{
+		{
+			&PolymorphicEvent{
+				CRTs: 1,
+			},
+			&PolymorphicEvent{
+				CRTs: 2,
+			},
+			true,
+		},
+		{
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType: OpTypeResolved,
+				},
+			},
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType: OpTypePut,
+				},
+			},
+			false,
+		},
+		{
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType: OpTypeResolved,
+				},
+			},
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType: OpTypeResolved,
+				},
+			},
+			false,
+		},
+		{
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType: OpTypePut,
+				},
+			},
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType: OpTypeResolved,
+				},
+			},
+			true,
+		},
+		{
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType:   OpTypePut,
+					Sequence: 1,
+				},
+			},
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType:   OpTypePut,
+					Sequence: 2,
+				},
+			},
+			true,
+		},
+		{
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType:   OpTypePut,
+					Sequence: 2,
+				},
+			},
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType:   OpTypePut,
+					Sequence: 1,
+				},
+			},
+			false,
+		},
+		{
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType:   OpTypeResolved,
+					Sequence: 1,
+				},
+			},
+			&PolymorphicEvent{
+				CRTs: 2,
+				RawKV: &RawKVEntry{
+					OpType:   OpTypePut,
+					Sequence: 2,
+				},
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		require.Equal(t, tc.expected, ComparePolymorphicEvents(tc.i, tc.j))
+	}
+}
diff --git a/cdc/cdc/puller/puller.go b/cdc/cdc/puller/puller.go
@@ -58,6 +58,7 @@ type pullerImpl struct {
 	resolvedTs     uint64
 	initialized    int64
 	enableOldValue bool
+	eventSeq       uint64
 }
 
 // NewPuller create a new Puller fetch event start from checkpointTs
@@ -95,6 +96,7 @@ func NewPuller(
 		resolvedTs:     checkpointTs,
 		initialized:    0,
 		enableOldValue: enableOldValue,
+		eventSeq:       0,
 	}
 	return p
 }
@@ -185,6 +187,8 @@ func (p *pullerImpl) Run(ctx context.Context) error {
 			log.Debug("[TRACE] revcive region feed event", zap.Stringer("event", e))
 
 			if e.Val != nil {
+				e.Val.Sequence = p.eventSeq
+				p.eventSeq += 1
 				metricTxnCollectCounterKv.Inc()
 				if err := output(e.Val); err != nil {
 					return errors.Trace(err)

diff --git a/cdc/cdc/puller/puller_test.go b/cdc/cdc/puller/puller_test.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"sync"
+	"testing"
 
 	"github.com/pingcap/check"
 	"github.com/pingcap/errors"
@@ -39,6 +40,10 @@ type pullerSuite struct{}
 
 var _ = check.Suite(&pullerSuite{})
 
+func TestSuite(t *testing.T) {
+	check.TestingT(t)
+}
+
 type mockPdClientForPullerTest struct {
 	pd.Client
 	clusterID uint64
@@ -225,3 +230,35 @@ func (s *pullerSuite) TestPullerRawKV(c *check.C) {
 	cancel()
 	wg.Wait()
 }
+
+func (s *pullerSuite) TestAssignSequence(c *check.C) {
+	defer testleak.AfterTest(c)()
+	spans := []regionspan.Span{
+		{Start: []byte("t_a"), End: []byte("t_z")},
+	}
+	checkpointTs := uint64(999)
+	plr, cancel, wg, store := s.newPullerForTest(c, spans, checkpointTs)
+	go func() {
+		for i := 1; i <= 100; i++ {
+			plr.cli.Returns(model.RegionFeedEvent{
+				Val: &model.RawKVEntry{
+					OpType: model.OpTypePut,
+					Key:    []byte("t_b"),
+					Value:  []byte("test-value"),
+					CRTs:   checkpointTs + uint64(i),
+				},
+			})
+		}
+	}()
+
+	seq := uint64(0)
+	for seq < uint64(100) {
+		ev := <-plr.Output()
+		c.Assert(ev.Sequence, check.Equals, seq)
+		seq += 1
+	}
+
+	store.Close()
+	cancel()
+	wg.Wait()
+}