From b0253430e3074480ed5573bec2c18420d1e88578 Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Wed, 29 Jul 2026 09:40:34 +0800 Subject: [PATCH 01/11] fix(tae): persist and filter append abort metadata --- pkg/objectio/const.go | 153 ++++++++++++++---- pkg/objectio/funcs.go | 25 +-- pkg/objectio/funcs_test.go | 133 +++++++++++++++ pkg/objectio/ioutil/funcs.go | 31 +++- pkg/publication/filter_object_batch.go | 72 ++++++--- pkg/publication/filter_object_batch_test.go | 7 +- .../disttae/local_disttae_datasource.go | 5 + .../disttae/logtailreplay/change_handle.go | 10 +- .../disttae/logtailreplay/partition_state.go | 41 +++-- pkg/vm/engine/disttae/pk_check_guard_test.go | 10 +- pkg/vm/engine/disttae/txn_table.go | 23 ++- pkg/vm/engine/readutil/reader.go | 2 +- pkg/vm/engine/tae/blockio/read.go | 21 ++- pkg/vm/engine/tae/db/test/db_test.go | 4 +- .../engine/tae/index/indexwrapper/mutindex.go | 18 ++- pkg/vm/engine/tae/logtail/snapshot.go | 26 ++- pkg/vm/engine/tae/logtail/tools.go | 69 +++++--- pkg/vm/engine/tae/rpc/dump_table.go | 6 + pkg/vm/engine/tae/rpc/tool.go | 8 +- pkg/vm/engine/tae/tables/base.go | 47 ++++-- pkg/vm/engine/tae/tables/functions.go | 58 +++++-- pkg/vm/engine/tae/tables/functions_test.go | 64 ++++++-- .../engine/tae/tables/jobs/flushTableTail.go | 13 +- pkg/vm/engine/tae/tables/mnode.go | 39 ++++- pkg/vm/engine/tae/tables/object_test.go | 84 ++++++++++ pkg/vm/engine/tae/tables/pnode.go | 54 ++++--- pkg/vm/engine/tae/tables/updates/append.go | 33 +++- pkg/vm/engine/tae/tables/updates/cmd.go | 44 +++-- .../engine/tae/tables/updates/command_test.go | 26 +++ pkg/vm/engine/tae/tables/updates/mvcc.go | 6 - pkg/vm/engine/tae/tables/updates/mvcc_test.go | 20 +++ pkg/vm/engine/tae/tables/utils.go | 76 +++++---- 32 files changed, 959 insertions(+), 269 deletions(-) diff --git a/pkg/objectio/const.go b/pkg/objectio/const.go index c8fd8f91c598a..944fb17927d36 100644 --- a/pkg/objectio/const.go +++ b/pkg/objectio/const.go @@ -17,6 +17,7 @@ package objectio import ( "fmt" "math" + "slices" "github.com/matrixorigin/matrixone/pkg/container/types" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/index" @@ -74,6 +75,75 @@ const ( const HiddenColumnSelection_None HiddenColumnSelection = 0 +const InvalidSpecialColumnPosition = math.MaxUint16 + +// SpecialColumnLayout describes the physical metadata positions of the +// appendable-only columns. Object writers map special seqnums, in declaration +// order, immediately after MaxSeqnum. PhysicalAddr may therefore appear before +// or after CommitTS, depending on the writer. The positions are stable across +// sparse user schemas and do not depend on the total column count. +// +// Old appendable objects contain only CommitTS. New appendable objects contain +// CommitTS followed by Abort. +type SpecialColumnLayout struct { + PhysicalAddr uint16 + CommitTS uint16 + Abort uint16 +} + +func (layout SpecialColumnLayout) Resolve(seqnum uint16) (uint16, bool) { + switch seqnum { + case SEQNUM_COMMITTS: + return layout.CommitTS, layout.CommitTS != InvalidSpecialColumnPosition + case SEQNUM_ABORT: + return layout.Abort, layout.Abort != InvalidSpecialColumnPosition + default: + return InvalidSpecialColumnPosition, false + } +} + +// ResolveSpecialColumnLayout resolves appendable special columns by their +// format-defined positions and validates their types. This keeps old +// commitTS-only objects readable while preventing a user TS/bool column from +// being mistaken for a hidden column. +func ResolveSpecialColumnLayout(block BlockObject) SpecialColumnLayout { + layout := SpecialColumnLayout{ + PhysicalAddr: InvalidSpecialColumnPosition, + CommitTS: InvalidSpecialColumnPosition, + Abort: InvalidSpecialColumnPosition, + } + metaColumnCount := block.GetMetaColumnCount() + if metaColumnCount == 0 { + return layout + } + + pos := block.GetMaxSeqnum() + 1 + if pos < metaColumnCount && + block.ColumnMeta(pos).DataType() == uint8(types.T_Rowid) { + layout.PhysicalAddr = pos + pos++ + } + + commitPos := pos + if commitPos >= metaColumnCount || + block.ColumnMeta(commitPos).DataType() != uint8(types.T_TS) { + return layout + } + layout.CommitTS = commitPos + + abortPos := commitPos + 1 + if abortPos < metaColumnCount && + block.ColumnMeta(abortPos).DataType() == uint8(types.T_Rowid) { + layout.PhysicalAddr = abortPos + abortPos++ + } + if abortPos < metaColumnCount && + block.ColumnMeta(abortPos).DataType() == uint8(types.T_bool) { + layout.Abort = abortPos + } + return layout +} + var ( TombstoneSeqnums_CN_Created = []uint16{0, 1} TombstoneSeqnums_CN_Created_PhyAddr = []uint16{0, 1, SEQNUM_ROWID} @@ -97,41 +167,53 @@ func IsPhysicalAddr(attr string) bool { return attr == PhysicalAddr_Attr } +func normalizeTombstoneHiddenColumns(hidden HiddenColumnSelection) HiddenColumnSelection { + // Abort is part of appendable MVCC metadata and is never meaningful without + // the commit timestamp that defines the row's visibility interval. + if hidden&HiddenColumnSelection_Abort != 0 { + hidden |= HiddenColumnSelection_CommitTS + } + return hidden +} + func GetTombstoneAttrs(hidden HiddenColumnSelection) []string { + hidden = normalizeTombstoneHiddenColumns(hidden) + var attrs []string if hidden&HiddenColumnSelection_PhysicalAddr != 0 && hidden&HiddenColumnSelection_CommitTS != 0 { - return TombstoneAttrs_TN_Created_PhyAddr - } - if hidden&HiddenColumnSelection_PhysicalAddr != 0 { - return TombstoneAttrs_CN_Created_PhyAddr + attrs = TombstoneAttrs_TN_Created_PhyAddr + } else if hidden&HiddenColumnSelection_PhysicalAddr != 0 { + attrs = TombstoneAttrs_CN_Created_PhyAddr + } else if hidden&HiddenColumnSelection_CommitTS != 0 { + attrs = TombstoneAttrs_TN_Created + } else { + attrs = TombstoneAttrs_CN_Created } - if hidden&HiddenColumnSelection_CommitTS != 0 { - return TombstoneAttrs_TN_Created + attrs = slices.Clone(attrs) + if hidden&HiddenColumnSelection_Abort != 0 { + attrs = append(attrs, TombstoneAttr_Abort_Attr) } - return TombstoneAttrs_CN_Created -} - -func GetTombstoneCommitTSAttrIdx(columnCnt uint16) uint16 { - if columnCnt == 3 { - return TombstoneAttr_NA_CommitTs_Idx - } else if columnCnt == 4 { - return TombstoneAttr_A_CommitTs_Idx - } - panic(fmt.Sprintf("invalid tombstone column count %d", columnCnt)) + return attrs } func GetTombstoneSeqnums(hidden HiddenColumnSelection) []uint16 { + hidden = normalizeTombstoneHiddenColumns(hidden) + var seqnums []uint16 if hidden&HiddenColumnSelection_PhysicalAddr != 0 && hidden&HiddenColumnSelection_CommitTS != 0 { - return TombstoneSeqnums_DN_Created_PhyAddr - } - if hidden&HiddenColumnSelection_PhysicalAddr != 0 { - return TombstoneSeqnums_CN_Created_PhyAddr + seqnums = TombstoneSeqnums_DN_Created_PhyAddr + } else if hidden&HiddenColumnSelection_PhysicalAddr != 0 { + seqnums = TombstoneSeqnums_CN_Created_PhyAddr + } else if hidden&HiddenColumnSelection_CommitTS != 0 { + seqnums = TombstoneSeqnums_DN_Created + } else { + seqnums = TombstoneSeqnums_CN_Created } - if hidden&HiddenColumnSelection_CommitTS != 0 { - return TombstoneSeqnums_DN_Created + seqnums = slices.Clone(seqnums) + if hidden&HiddenColumnSelection_Abort != 0 { + seqnums = append(seqnums, SEQNUM_ABORT) } - return TombstoneSeqnums_CN_Created + return seqnums } func GetTombstoneSchema( @@ -144,33 +226,38 @@ func GetTombstoneSchema( func GetTombstoneTypes( pk types.Type, hidden HiddenColumnSelection, ) []types.Type { + hidden = normalizeTombstoneHiddenColumns(hidden) + var typs []types.Type if hidden&HiddenColumnSelection_PhysicalAddr != 0 && hidden&HiddenColumnSelection_CommitTS != 0 { - return []types.Type{ + typs = []types.Type{ RowidType, pk, TSType, RowidType, } - } - if hidden&HiddenColumnSelection_PhysicalAddr != 0 { - return []types.Type{ + } else if hidden&HiddenColumnSelection_PhysicalAddr != 0 { + typs = []types.Type{ RowidType, pk, RowidType, } - } - if hidden&HiddenColumnSelection_CommitTS != 0 { - return []types.Type{ + } else if hidden&HiddenColumnSelection_CommitTS != 0 { + typs = []types.Type{ RowidType, pk, TSType, } + } else { + typs = []types.Type{ + RowidType, + pk, + } } - return []types.Type{ - RowidType, - pk, + if hidden&HiddenColumnSelection_Abort != 0 { + typs = append(typs, types.T_bool.ToType()) } + return typs } func MustGetPhysicalColumnPosition(seqnums []uint16, colTypes []types.Type) int { diff --git a/pkg/objectio/funcs.go b/pkg/objectio/funcs.go index 917ac5fecab38..15bfff39722a5 100644 --- a/pkg/objectio/funcs.go +++ b/pkg/objectio/funcs.go @@ -168,32 +168,19 @@ func ReadOneBlockWithMeta( blkmeta := meta.GetBlockMeta(uint32(blk)) maxSeqnum := blkmeta.GetMaxSeqnum() + specialLayout := ResolveSpecialColumnLayout(blkmeta) for i, seqnum := range seqnums { // special columns if seqnum >= SEQNUM_UPPER { - metaColCnt := blkmeta.GetMetaColumnCount() - switch seqnum { - case SEQNUM_COMMITTS: - if metaColCnt == 0 { - putFillHolder(i, 0) - continue - } - seqnum = metaColCnt - 1 - case SEQNUM_ABORT: - panic("not support") - default: + var ok bool + if seqnum != SEQNUM_COMMITTS && seqnum != SEQNUM_ABORT { panic(fmt.Sprintf("bad path to read special column %d", seqnum)) } - // Type alone is insufficient: the last user column may itself be - // T_TS. A hidden commit-TS column must sit beyond MaxSeqnum. - // If the last column is not commits, do not read it: - // 1. created by cn - // 2. old version tn nonappendable block - col := blkmeta.ColumnMeta(seqnum) - hasHiddenColumn := metaColCnt > maxSeqnum+1 - if !hasHiddenColumn || col.DataType() != uint8(types.T_TS) { + seqnum, ok = specialLayout.Resolve(seqnum) + if !ok { putFillHolder(i, seqnum) } else { + col := blkmeta.ColumnMeta(seqnum) ext := col.Location() ioVec.Entries = append(ioVec.Entries, fileservice.IOEntry{ Offset: int64(ext.Offset()), diff --git a/pkg/objectio/funcs_test.go b/pkg/objectio/funcs_test.go index a5b6f6c53bb10..a499d24cab31a 100644 --- a/pkg/objectio/funcs_test.go +++ b/pkg/objectio/funcs_test.go @@ -179,6 +179,139 @@ func TestReadOneBlockSynthesizesCommitTSForEmptyMetadata(t *testing.T) { ioVec.ReleaseReadResultOnError() } +func TestResolveSpecialColumnLayoutCompatibility(t *testing.T) { + t.Run("commit-only", func(t *testing.T) { + block := NewBlock(NewSeqnums([]uint16{0, SEQNUM_COMMITTS})) + block.ColumnMeta(0).setDataType(uint8(types.T_int64)) + block.ColumnMeta(1).setDataType(uint8(types.T_TS)) + + layout := ResolveSpecialColumnLayout(block) + require.Equal(t, uint16(1), layout.CommitTS) + require.Equal(t, uint16(InvalidSpecialColumnPosition), layout.Abort) + }) + + t.Run("commit-and-abort-with-sparse-schema", func(t *testing.T) { + block := NewBlock(NewSeqnums([]uint16{3, SEQNUM_COMMITTS, SEQNUM_ABORT})) + block.ColumnMeta(3).setDataType(uint8(types.T_int64)) + block.ColumnMeta(4).setDataType(uint8(types.T_TS)) + block.ColumnMeta(5).setDataType(uint8(types.T_bool)) + + layout := ResolveSpecialColumnLayout(block) + require.Equal(t, uint16(4), layout.CommitTS) + require.Equal(t, uint16(5), layout.Abort) + }) + + t.Run("physical-address-before-mvcc", func(t *testing.T) { + block := NewBlock(NewSeqnums([]uint16{ + 3, + SEQNUM_ROWID, + SEQNUM_COMMITTS, + SEQNUM_ABORT, + })) + block.ColumnMeta(3).setDataType(uint8(types.T_int64)) + block.ColumnMeta(4).setDataType(uint8(types.T_Rowid)) + block.ColumnMeta(5).setDataType(uint8(types.T_TS)) + block.ColumnMeta(6).setDataType(uint8(types.T_bool)) + + layout := ResolveSpecialColumnLayout(block) + require.Equal(t, uint16(4), layout.PhysicalAddr) + require.Equal(t, uint16(5), layout.CommitTS) + require.Equal(t, uint16(6), layout.Abort) + }) + + t.Run("physical-address-between-commit-and-abort", func(t *testing.T) { + block := NewBlock(NewSeqnums([]uint16{ + 1, + SEQNUM_COMMITTS, + SEQNUM_ROWID, + SEQNUM_ABORT, + })) + block.ColumnMeta(1).setDataType(uint8(types.T_int64)) + block.ColumnMeta(2).setDataType(uint8(types.T_TS)) + block.ColumnMeta(3).setDataType(uint8(types.T_Rowid)) + block.ColumnMeta(4).setDataType(uint8(types.T_bool)) + + layout := ResolveSpecialColumnLayout(block) + require.Equal(t, uint16(3), layout.PhysicalAddr) + require.Equal(t, uint16(2), layout.CommitTS) + require.Equal(t, uint16(4), layout.Abort) + }) + + t.Run("user-columns-are-not-special", func(t *testing.T) { + block := NewBlock(NewSeqnums([]uint16{0, 1})) + block.ColumnMeta(0).setDataType(uint8(types.T_TS)) + block.ColumnMeta(1).setDataType(uint8(types.T_bool)) + + layout := ResolveSpecialColumnLayout(block) + require.Equal(t, uint16(InvalidSpecialColumnPosition), layout.CommitTS) + require.Equal(t, uint16(InvalidSpecialColumnPosition), layout.Abort) + }) +} + +func TestTombstoneAbortSelectionIncludesCommitTS(t *testing.T) { + hidden := HiddenColumnSelection_Abort + require.Equal( + t, + []uint16{0, 1, SEQNUM_COMMITTS, SEQNUM_ABORT}, + GetTombstoneSeqnums(hidden), + ) + require.Equal( + t, + []string{ + TombstoneAttr_Rowid_Attr, + TombstoneAttr_PK_Attr, + TombstoneAttr_CommitTs_Attr, + TombstoneAttr_Abort_Attr, + }, + GetTombstoneAttrs(hidden), + ) +} + +func TestReadOneBlockAbortColumnCompatibility(t *testing.T) { + t.Run("new-object-reads-abort", func(t *testing.T) { + readErr := moerr.NewInternalErrorNoCtx("abort column storage read") + fs := &partialReadErrorFS{err: readErr} + meta := BuildMetaData(1, 3) + block := meta.GetBlockMeta(0) + block.BlockHeader().SetRows(1) + block.BlockHeader().SetMaxSeqnum(0) + block.BlockHeader().SetMetaColumnCount(3) + block.ColumnMeta(0).setDataType(uint8(types.T_int64)) + block.ColumnMeta(1).setDataType(uint8(types.T_TS)) + block.ColumnMeta(2).setDataType(uint8(types.T_bool)) + block.ColumnMeta(2).setLocation(NewExtent(1, 0, 1, 1)) + + _, err := ReadOneBlockWithMeta( + context.Background(), &meta, "new-object", 0, + []uint16{SEQNUM_ABORT}, []types.Type{types.T_bool.ToType()}, + mpool.MustNewZero(), fs, constructorFactory, fileservice.Policy(0), + ) + require.ErrorIs(t, err, readErr) + }) + + t.Run("old-object-synthesizes-missing-abort", func(t *testing.T) { + readErr := moerr.NewInternalErrorNoCtx("must not read storage") + fs := &partialReadErrorFS{err: readErr} + meta := BuildMetaData(1, 2) + block := meta.GetBlockMeta(0) + block.BlockHeader().SetRows(1) + block.BlockHeader().SetMaxSeqnum(0) + block.BlockHeader().SetMetaColumnCount(2) + block.ColumnMeta(0).setDataType(uint8(types.T_int64)) + block.ColumnMeta(1).setDataType(uint8(types.T_TS)) + + ioVec, err := ReadOneBlockWithMeta( + context.Background(), &meta, "old-object", 0, + []uint16{SEQNUM_ABORT}, []types.Type{types.T_bool.ToType()}, + mpool.MustNewZero(), fs, constructorFactory, fileservice.Policy(0), + ) + require.NoError(t, err) + require.Len(t, ioVec.Entries, 1) + require.NotNil(t, ioVec.Entries[0].CachedData) + ioVec.ReleaseReadResultOnError() + }) +} + func TestReadAllBlocksWithMetaReleasesPartialReadOnError(t *testing.T) { var releases atomic.Int32 readErr := moerr.NewInternalErrorNoCtx("read canceled after partial all-blocks fill") diff --git a/pkg/objectio/ioutil/funcs.go b/pkg/objectio/ioutil/funcs.go index 6aa8522dc49db..68504f5dd8017 100644 --- a/pkg/objectio/ioutil/funcs.go +++ b/pkg/objectio/ioutil/funcs.go @@ -349,11 +349,16 @@ func IsRowDeletedByLocation( deleted = (idx < len(rowids)) && (rowids[idx].EQ(row)) } else { tss := vector.MustFixedColNoTypeCheck[types.TS](&data[1]) + abortVec := &data[2] + var aborts []bool + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColNoTypeCheck[bool](abortVec) + } for i := idx; i < len(rowids); i++ { if !rowids[i].EQ(row) { break } - if tss[i].LE(snapshotTS) { + if (aborts == nil || !aborts[i]) && tss[i].LE(snapshotTS) { deleted = true break } @@ -400,6 +405,7 @@ func FillBlockDeleteMask( deleteMask = EvalDeleteMaskFromDNCreatedTombstones( &persistedDeletes[0], &persistedDeletes[1], + &persistedDeletes[2], meta.GetBlockMeta(uint32(location.ID())), snapshotTS, blockId, @@ -426,8 +432,12 @@ func ReadDeletes( cols = []uint16{objectio.TombstoneAttr_Rowid_SeqNum} typs = []types.Type{objectio.RowidType} } else { - cols = []uint16{objectio.TombstoneAttr_Rowid_SeqNum, objectio.TombstoneAttr_CommitTs_SeqNum} - typs = []types.Type{objectio.RowidType, objectio.TSType} + cols = []uint16{ + objectio.TombstoneAttr_Rowid_SeqNum, + objectio.TombstoneAttr_CommitTs_SeqNum, + objectio.TombstoneAttr_Abort_SeqNum, + } + typs = []types.Type{objectio.RowidType, objectio.TSType, types.T_bool.ToType()} } if pkType != nil { @@ -443,6 +453,7 @@ func ReadDeletes( func EvalDeleteMaskFromDNCreatedTombstones( deletedRows *vector.Vector, commitTSVec *vector.Vector, + abortVec *vector.Vector, meta objectio.BlockObject, ts *types.TS, blockid *types.Blockid, @@ -457,11 +468,17 @@ func EvalDeleteMaskFromDNCreatedTombstones( } noTSCheck := false - if end-start > 10 { + var aborts []bool + if abortVec != nil && !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + } + if end-start > 10 && aborts == nil { // fast path is true if the maxTS is less than the snapshotTS // this means that all the rows between start and end are visible - idx := objectio.GetTombstoneCommitTSAttrIdx(meta.GetMetaColumnCount()) - noTSCheck = meta.MustGetColumn(idx).ZoneMap().FastLEValue(ts[:], 0) + layout := objectio.ResolveSpecialColumnLayout(meta) + if idx, ok := layout.Resolve(objectio.SEQNUM_COMMITTS); ok { + noTSCheck = meta.MustGetColumn(idx).ZoneMap().FastLEValue(ts[:], 0) + } } rows = objectio.GetReusableBitmap() if noTSCheck { @@ -472,7 +489,7 @@ func EvalDeleteMaskFromDNCreatedTombstones( } else { tss := vector.MustFixedColWithTypeCheck[types.TS](commitTSVec) for i := end - 1; i >= start; i-- { - if tss[i].GT(ts) { + if (aborts != nil && aborts[i]) || tss[i].GT(ts) { continue } row := rowids[i].GetRowOffset() diff --git a/pkg/publication/filter_object_batch.go b/pkg/publication/filter_object_batch.go index 5bac51b3f1b0c..0a7a3570af6fc 100644 --- a/pkg/publication/filter_object_batch.go +++ b/pkg/publication/filter_object_batch.go @@ -256,8 +256,8 @@ func convertObjectToBatch( // Step 3: Prepare columns and types // For appendable objects, we need to include commit TS column - cols := make([]uint16, 0, maxSeqnum+2) - typs := make([]types.Type, 0, maxSeqnum+2) + cols := make([]uint16, 0, maxSeqnum+3) + typs := make([]types.Type, 0, maxSeqnum+3) // Add data columns for seqnum := uint16(0); seqnum <= maxSeqnum; seqnum++ { @@ -273,6 +273,8 @@ func convertObjectToBatch( // Add commit TS column for appendable objects cols = append(cols, objectio.SEQNUM_COMMITTS) typs = append(typs, objectio.TSType) + cols = append(cols, objectio.SEQNUM_ABORT) + typs = append(typs, types.T_bool.ToType()) // Step 4: Read column data from ALL blocks and merge // Initialize vectors for each column @@ -280,6 +282,16 @@ func convertObjectToBatch( for i, typ := range typs { vecs[i] = containers.MakeVector(typ, mp) } + ownershipTransferred := false + defer func() { + if !ownershipTransferred { + for _, vec := range vecs { + if vec != nil { + vec.Close() + } + } + } + }() allocator := fileservice.DefaultCacheDataAllocator() // Iterate through all blocks @@ -290,14 +302,19 @@ func convertObjectToBatch( var colMeta objectio.ColumnMeta var ext objectio.Extent - // Handle special columns (commit TS) + // Handle appendable special columns through the shared layout. if seqnum >= objectio.SEQNUM_UPPER { - if seqnum == objectio.SEQNUM_COMMITTS { - metaColCnt := blkMeta.GetMetaColumnCount() - colMeta = blkMeta.ColumnMeta(metaColCnt - 1) - } else { + pos, ok := objectio.ResolveSpecialColumnLayout(blkMeta).Resolve(seqnum) + if !ok { + if seqnum == objectio.SEQNUM_ABORT { + for range int(blkMeta.GetRows()) { + vecs[i].Append(false, false) + } + continue + } return nil, moerr.NewInternalErrorf(ctx, "unsupported special column: %d", seqnum) } + colMeta = blkMeta.ColumnMeta(pos) } else { // Normal column if seqnum > maxSeqnum || blkMeta.ColumnMeta(seqnum).DataType() == 0 { @@ -365,12 +382,15 @@ func convertObjectToBatch( var attr string if cols[i] == objectio.SEQNUM_COMMITTS { attr = objectio.TombstoneAttr_CommitTs_Attr + } else if cols[i] == objectio.SEQNUM_ABORT { + attr = objectio.TombstoneAttr_Abort_Attr } else { attr = fmt.Sprintf("tmp_%d", i) } bat.AddVector(attr, vec) } + ownershipTransferred = true return bat, nil } @@ -399,11 +419,27 @@ func filterBatchBySnapshotTS( // Get commit TS values commitTSs := vector.MustFixedColWithTypeCheck[types.TS](commitTSVec.GetDownstreamVector()) + var aborts []bool + if abortPos, ok := bat.Nameidx[objectio.TombstoneAttr_Abort_Attr]; ok { + abortVec := bat.Vecs[abortPos] + if abortVec.GetType().Oid != types.T_bool { + return nil, moerr.NewInternalErrorf(ctx, "abort column has invalid type") + } + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec.GetDownstreamVector()) + if len(aborts) != len(commitTSs) { + return nil, moerr.NewInternalErrorf( + ctx, + "abort column length %d does not match commit TS length %d", + len(aborts), + len(commitTSs), + ) + } + } // Build bitmap of rows to delete (commit TS < snapshot TS) deletes := roaring.New() for i, ts := range commitTSs { - if ts.GT(&snapshotTS) { + if ts.GT(&snapshotTS) || (aborts != nil && aborts[i]) { deletes.Add(uint32(i)) } } @@ -471,26 +507,26 @@ func createObjectFromBatch( } } - // Step 3: Remove commit TS column - // Find commit TS column index - commitTSIdx := -1 + // Step 3: Remove appendable-only MVCC columns. + commitTSIdx, abortIdx := -1, -1 for i, attr := range cnBat.Attrs { if attr == objectio.TombstoneAttr_CommitTs_Attr { commitTSIdx = i - break + } else if attr == objectio.TombstoneAttr_Abort_Attr { + abortIdx = i } } - if commitTSIdx == -1 { - return objectio.ObjectStats{}, nil, moerr.NewInternalErrorf(ctx, "commit TS column not found") + if commitTSIdx == -1 || abortIdx == -1 { + return objectio.ObjectStats{}, nil, moerr.NewInternalErrorf(ctx, "MVCC columns not found") } - // Create new batch without commit TS column + // Create new batch without commit TS and abort. newBat := &batch.Batch{ - Vecs: make([]*vector.Vector, 0, len(cnBat.Vecs)-1), - Attrs: make([]string, 0, len(cnBat.Attrs)-1), + Vecs: make([]*vector.Vector, 0, len(cnBat.Vecs)-2), + Attrs: make([]string, 0, len(cnBat.Attrs)-2), } for i, vec := range cnBat.Vecs { - if i != commitTSIdx { + if i != commitTSIdx && i != abortIdx { newBat.Attrs = append(newBat.Attrs, cnBat.Attrs[i]) newBat.Vecs = append(newBat.Vecs, vec) } diff --git a/pkg/publication/filter_object_batch_test.go b/pkg/publication/filter_object_batch_test.go index ce1ea84bca730..f5f9101a0bd00 100644 --- a/pkg/publication/filter_object_batch_test.go +++ b/pkg/publication/filter_object_batch_test.go @@ -173,11 +173,16 @@ func TestFilterBatchBySnapshotTS_WithDeletes(t *testing.T) { tsVec.Append(ts2, false) tsVec.Append(ts3, false) bat.AddVector(objectio.TombstoneAttr_CommitTs_Attr, tsVec) + abortVec := containers.MakeVector(types.T_bool.ToType(), mp) + abortVec.Append(false, false) + abortVec.Append(false, false) + abortVec.Append(true, false) // visible commit TS, but rolled back + bat.AddVector(objectio.TombstoneAttr_Abort_Attr, abortVec) snapshotTS := types.BuildTS(300, 0) result, err := filterBatchBySnapshotTS(context.Background(), bat, snapshotTS, mp) assert.NoError(t, err) - assert.Equal(t, 2, result.Length()) + assert.Equal(t, 1, result.Length()) bat.Close() } diff --git a/pkg/vm/engine/disttae/local_disttae_datasource.go b/pkg/vm/engine/disttae/local_disttae_datasource.go index 88da457d98399..ccacab6f09f26 100644 --- a/pkg/vm/engine/disttae/local_disttae_datasource.go +++ b/pkg/vm/engine/disttae/local_disttae_datasource.go @@ -1562,10 +1562,14 @@ func (ls *LocalDisttaeDataSource) batchApplyTombstoneObjects( var deletedRowIds []objectio.Rowid var commit []types.TS + var aborts []bool deletedRowIds = vector.MustFixedColWithTypeCheck[objectio.Rowid](&cacheVectors[0]) if !obj.GetCNCreated() { commit = vector.MustFixedColWithTypeCheck[types.TS](&cacheVectors[1]) + if !cacheVectors[2].IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](&cacheVectors[2]) + } } for i := 0; i < len(rowIds); i++ { @@ -1574,6 +1578,7 @@ func (ls *LocalDisttaeDataSource) batchApplyTombstoneObjects( for j := s; j < e; j++ { if rowIds[i].EQ(&deletedRowIds[j]) && + (aborts == nil || !aborts[j]) && (commit == nil || commit[j].LE(&ls.snapshotTS)) { deletedMask.Add(uint64(i)) break diff --git a/pkg/vm/engine/disttae/logtailreplay/change_handle.go b/pkg/vm/engine/disttae/logtailreplay/change_handle.go index 9eb69f9bbdb99..8d5c3e3eeb274 100755 --- a/pkg/vm/engine/disttae/logtailreplay/change_handle.go +++ b/pkg/vm/engine/disttae/logtailreplay/change_handle.go @@ -652,14 +652,12 @@ func blockCommitTSOverlapsRange( if metaColCnt == 0 { return false, false, "no_meta_columns", base } - // Commit-ts is stored as the trailing hidden column when available. - // Do not gate by max-seqnum here: merged/rewritten TN objects may expose - // different seqnum layouts while still carrying valid commit-ts zonemap. - commitCol := blk.ColumnMeta(metaColCnt - 1) - base = fmt.Sprintf("%s tail_col_type=%d", base, commitCol.DataType()) - if commitCol.DataType() != uint8(types.T_TS) { + commitPos, ok := objectio.ResolveSpecialColumnLayout(blk).Resolve(objectio.SEQNUM_COMMITTS) + if !ok { return false, false, "tail_column_not_ts", base } + commitCol := blk.ColumnMeta(commitPos) + base = fmt.Sprintf("%s commit_pos=%d", base, commitPos) zm := commitCol.ZoneMap() if !zm.IsInited() { return false, false, "zonemap_not_inited", base diff --git a/pkg/vm/engine/disttae/logtailreplay/partition_state.go b/pkg/vm/engine/disttae/logtailreplay/partition_state.go index 4919c8443fcaf..e804aeda29b95 100644 --- a/pkg/vm/engine/disttae/logtailreplay/partition_state.go +++ b/pkg/vm/engine/disttae/logtailreplay/partition_state.go @@ -1277,9 +1277,9 @@ func (p *PartitionState) countVisibleRowsInAppendableObject( obj objectio.ObjectEntry, mp *mpool.MPool, ) (uint64, error) { - cols := []uint16{objectio.SEQNUM_COMMITTS} - typs := []types.Type{types.T_TS.ToType()} - cacheVectors := containers.NewVectors(1) + cols := []uint16{objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT} + typs := []types.Type{types.T_TS.ToType(), types.T_bool.ToType()} + cacheVectors := containers.NewVectors(2) var count uint64 var loadErr error @@ -1296,8 +1296,13 @@ func (p *PartitionState) countVisibleRowsInAppendableObject( return true } commitTSCol := vector.MustFixedColWithTypeCheck[types.TS](&cacheVectors[0]) - for _, ts := range commitTSCol { - if ts.LE(&snapshot) { + abortVec := &cacheVectors[1] + var aborts []bool + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + } + for row, ts := range commitTSCol { + if (aborts == nil || !aborts[row]) && ts.LE(&snapshot) { count++ } } @@ -1645,10 +1650,14 @@ func (p *PartitionState) countTombstoneStatsLinear( rowIds := vector.MustFixedColNoTypeCheck[types.Rowid](&persistedDeletes[0]) var commitTSs []types.TS + var aborts []bool // When cnCreated=false (TN created), ReadDeletes reads [Rowid, CommitTS] at indices [0, 1] // When cnCreated=true (CN created), ReadDeletes only reads [Rowid] at index [0], no CommitTS if needCheckCommitTs && len(persistedDeletes) > 2 { commitTSs = vector.MustFixedColNoTypeCheck[types.TS](&persistedDeletes[1]) + if !persistedDeletes[2].IsConstNull() { + aborts = vector.MustFixedColNoTypeCheck[bool](&persistedDeletes[2]) + } } var lastObjId types.Objectid @@ -1661,7 +1670,8 @@ func (p *PartitionState) countTombstoneStatsLinear( continue } - if needCheckCommitTs && len(commitTSs) > 0 && commitTSs[j].GT(&snapshot) { + if (aborts != nil && aborts[j]) || + (needCheckCommitTs && len(commitTSs) > 0 && commitTSs[j].GT(&snapshot)) { continue } @@ -1744,10 +1754,14 @@ func (p *PartitionState) countTombstoneStatsWithMap( rowIds := vector.MustFixedColNoTypeCheck[types.Rowid](&persistedDeletes[0]) var commitTSs []types.TS + var aborts []bool // When cnCreated=false (TN created), ReadDeletes reads [Rowid, CommitTS] at indices [0, 1] // When cnCreated=true (CN created), ReadDeletes only reads [Rowid] at index [0], no CommitTS if needCheckCommitTs && len(persistedDeletes) > 2 { commitTSs = vector.MustFixedColNoTypeCheck[types.TS](&persistedDeletes[1]) + if !persistedDeletes[2].IsConstNull() { + aborts = vector.MustFixedColNoTypeCheck[bool](&persistedDeletes[2]) + } } var lastObjId types.Objectid @@ -1759,7 +1773,8 @@ func (p *PartitionState) countTombstoneStatsWithMap( continue } - if needCheckCommitTs && len(commitTSs) > 0 && commitTSs[j].GT(&snapshot) { + if (aborts != nil && aborts[j]) || + (needCheckCommitTs && len(commitTSs) > 0 && commitTSs[j].GT(&snapshot)) { continue } @@ -1838,6 +1853,7 @@ type tombstoneBlockIterator struct { blockIdx int rowIds []types.Rowid commitTSs []types.TS + aborts []bool rowIdx int needCheckTS bool snapshot types.TS @@ -1876,9 +1892,15 @@ func (it *tombstoneBlockIterator) loadNextBlock() bool { it.rowIds = vector.MustFixedColNoTypeCheck[types.Rowid](&it.persistedDel[0]) if it.needCheckTS && len(it.persistedDel) > 2 { - it.commitTSs = vector.MustFixedColNoTypeCheck[types.TS](&it.persistedDel[len(it.persistedDel)-1]) + it.commitTSs = vector.MustFixedColNoTypeCheck[types.TS](&it.persistedDel[1]) + if !it.persistedDel[2].IsConstNull() { + it.aborts = vector.MustFixedColNoTypeCheck[bool](&it.persistedDel[2]) + } else { + it.aborts = nil + } } else { it.commitTSs = nil + it.aborts = nil } it.rowIdx = 0 @@ -1910,7 +1932,8 @@ func (it *tombstoneBlockIterator) next() bool { // Persisted tombstone iterator for { for it.rowIdx < len(it.rowIds) { - if it.needCheckTS && len(it.commitTSs) > 0 && it.commitTSs[it.rowIdx].GT(&it.snapshot) { + if (it.aborts != nil && it.aborts[it.rowIdx]) || + (it.needCheckTS && len(it.commitTSs) > 0 && it.commitTSs[it.rowIdx].GT(&it.snapshot)) { it.rowIdx++ continue } diff --git a/pkg/vm/engine/disttae/pk_check_guard_test.go b/pkg/vm/engine/disttae/pk_check_guard_test.go index fd4e76dae82e8..7c66d8440f8a6 100644 --- a/pkg/vm/engine/disttae/pk_check_guard_test.go +++ b/pkg/vm/engine/disttae/pk_check_guard_test.go @@ -156,26 +156,26 @@ func TestPKCommitTSMatchedInRange(t *testing.T) { require.NoError(t, vector.AppendFixed(vec, types.BuildTS(20, 0), false, mp)) require.NoError(t, vector.AppendFixed(vec, types.BuildTS(21, 0), false, mp)) - changed, ok := pkCommitTSMatchedInRange(vec, []int64{0, 3}, from, to) + changed, ok := pkCommitTSMatchedInRange(vec, nil, []int64{0, 3}, from, to) require.True(t, ok) require.False(t, changed) - changed, ok = pkCommitTSMatchedInRange(vec, []int64{1}, from, to) + changed, ok = pkCommitTSMatchedInRange(vec, nil, []int64{1}, from, to) require.True(t, ok) require.True(t, changed) - changed, ok = pkCommitTSMatchedInRange(vec, []int64{2}, from, to) + changed, ok = pkCommitTSMatchedInRange(vec, nil, []int64{2}, from, to) require.True(t, ok) require.True(t, changed) vec.SetNull(1) - changed, ok = pkCommitTSMatchedInRange(vec, []int64{1}, from, to) + changed, ok = pkCommitTSMatchedInRange(vec, nil, []int64{1}, from, to) require.False(t, ok) require.False(t, changed) constNull := vector.NewConstNull(types.T_TS.ToType(), 1, mp) defer constNull.Free(mp) - changed, ok = pkCommitTSMatchedInRange(constNull, []int64{0}, from, to) + changed, ok = pkCommitTSMatchedInRange(constNull, nil, []int64{0}, from, to) require.False(t, ok) require.False(t, changed) } diff --git a/pkg/vm/engine/disttae/txn_table.go b/pkg/vm/engine/disttae/txn_table.go index 1534259d8ff4d..3df437b110312 100644 --- a/pkg/vm/engine/disttae/txn_table.go +++ b/pkg/vm/engine/disttae/txn_table.go @@ -2614,6 +2614,7 @@ func (tbl *txnTable) getPartitionState( // callers should keep the original conservative behavior in that case. func pkCommitTSMatchedInRange( commitTSVec *vector.Vector, + abortVec *vector.Vector, sels []int64, from, to types.TS, ) (bool, bool) { @@ -2623,6 +2624,13 @@ func pkCommitTSMatchedInRange( return false, false } timestamps := vector.MustFixedColWithTypeCheck[types.TS](commitTSVec) + var aborts []bool + if abortVec != nil && !abortVec.IsConstNull() { + if abortVec.GetType().Oid != types.T_bool { + return false, false + } + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + } for _, sel := range sels { if sel < 0 || int(sel) >= len(timestamps) { return false, false @@ -2630,6 +2638,9 @@ func pkCommitTSMatchedInRange( if commitTSVec.IsNull(uint64(sel)) { return false, false } + if aborts != nil && aborts[sel] { + continue + } ts := timestamps[sel] if ts.GT(&from) && ts.LE(&to) { return true, true @@ -2817,7 +2828,7 @@ func (tbl *txnTable) PKPersistedBetween( return true, nil } - cacheVectors := containers.NewVectors(2) + cacheVectors := containers.NewVectors(3) pkDef := tbl.tableDef.Cols[tbl.primaryIdx] pkSeq := pkDef.Seqnum pkType := plan2.ExprType2Type(&pkDef.Typ) @@ -2835,8 +2846,8 @@ func (tbl *txnTable) PKPersistedBetween( for _, blk := range candidateBlks { release, _, err := ioutil.LoadColumns( ctx, - []uint16{uint16(pkSeq), objectio.SEQNUM_COMMITTS}, - []types.Type{pkType, types.T_TS.ToType()}, + []uint16{uint16(pkSeq), objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT}, + []types.Type{pkType, types.T_TS.ToType(), types.T_bool.ToType()}, fs, blk.MetaLocation(), cacheVectors, @@ -2856,7 +2867,7 @@ func (tbl *txnTable) PKPersistedBetween( sels := searchFunc(cacheVectors) if len(sels) > 0 { - changed, ok := pkCommitTSMatchedInRange(&cacheVectors[1], sels, from, to) + changed, ok := pkCommitTSMatchedInRange(&cacheVectors[1], &cacheVectors[2], sels, from, to) release() if !ok || changed { releasePKCheckSemaphore() @@ -2914,7 +2925,7 @@ func tombstonePKExistsInRange( for blkIdx := uint32(0); blkIdx < obj.BlkCnt(); blkIdx++ { loc := obj.BlockLocation(uint16(blkIdx), objectio.BlockMaxRows) isCNCreated := obj.GetCNCreated() - vecCount := 3 + vecCount := 4 if isCNCreated { vecCount = 2 } @@ -2930,7 +2941,7 @@ func tombstonePKExistsInRange( release() return true, "tombstone_cn_hit", nil } - changed, ok := pkCommitTSMatchedInRange(&tombVectors[2], hits, from, to) + changed, ok := pkCommitTSMatchedInRange(&tombVectors[2], &tombVectors[3], hits, from, to) release() if !ok || changed { if ok { diff --git a/pkg/vm/engine/readutil/reader.go b/pkg/vm/engine/readutil/reader.go index c184c320ca912..1977773a1f853 100644 --- a/pkg/vm/engine/readutil/reader.go +++ b/pkg/vm/engine/readutil/reader.go @@ -775,7 +775,7 @@ func (r *reader) Read( r.readBlockCnt++ if len(r.cacheVectors) == 0 { - r.cacheVectors = containers.NewVectors(len(r.columns.seqnums) + 1) + r.cacheVectors = containers.NewVectors(len(r.columns.seqnums) + 2) } if r.orderByLimit != nil && !r.orderByLimit.OrderedLimit && detachedDistVec != nil { // Re-attach the detached distVec so BlockDataRead can take its fast reuse branch. diff --git a/pkg/vm/engine/tae/blockio/read.go b/pkg/vm/engine/tae/blockio/read.go index b435c6c5a97cc..39f99fe49e9c7 100644 --- a/pkg/vm/engine/tae/blockio/read.go +++ b/pkg/vm/engine/tae/blockio/read.go @@ -149,7 +149,7 @@ func BlockDataReadNoCopy( } }() - cacheVectors := containers.NewVectors(len(columns) + 1) + cacheVectors := containers.NewVectors(len(columns) + 2) phyAddrColumnPos := -1 for i := range columns { @@ -835,11 +835,12 @@ func readBlockData( deletes objectio.Bitmap, err2 error, ) { - // appendable block should be filtered by committs - //cols = append(cols, objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT) // committs, aborted - cols = append(cols, objectio.SEQNUM_COMMITTS) // committs, aborted + // Appendable blocks are filtered by both MVCC special columns. The + // object reader synthesizes a NULL abort vector for old commitTS-only + // objects, which is interpreted as "no aborted rows". + cols = append(cols, objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT) + typs = append(typs, objectio.TSType, types.T_bool.ToType()) - // no need to add typs, the two columns won't be generated if err2 = readColumns( cols, cacheVectors2, ); err2 != nil { @@ -849,10 +850,14 @@ func readBlockData( deletes = objectio.GetReusableBitmap() t0 := time.Now() - //aborts := vector.MustFixedColWithTypeCheck[bool](loaded.Vecs[len(loaded.Vecs)-1]) - commits := vector.MustFixedColWithTypeCheck[types.TS](&cacheVectors2[len(cols)-1]) + abortVec := &cacheVectors2[len(cols)-1] + commits := vector.MustFixedColWithTypeCheck[types.TS](&cacheVectors2[len(cols)-2]) + var aborts []bool + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + } for i := 0; i < len(commits); i++ { - if commits[i].GT(&ts) { + if commits[i].GT(&ts) || (aborts != nil && aborts[i]) { deletes.Add(uint64(i)) } } diff --git a/pkg/vm/engine/tae/db/test/db_test.go b/pkg/vm/engine/tae/db/test/db_test.go index 84e6432735f4c..e4544113708bb 100644 --- a/pkg/vm/engine/tae/db/test/db_test.go +++ b/pkg/vm/engine/tae/db/test/db_test.go @@ -4986,7 +4986,7 @@ func TestBlockRead(t *testing.T) { } b1 := buildBatch(colTyps) phyAddrColumnPos := -1 - cacheVectors := containers.NewVectors(len(colIdxs) + 1) + cacheVectors := containers.NewVectors(len(colIdxs) + 2) err = blockio.BlockDataReadInner( context.Background(), info, ds, colIdxs, colTyps, phyAddrColumnPos, beforeDel, nil, nil, fileservice.Policy(0), b1, cacheVectors, pool, fs, @@ -5134,7 +5134,7 @@ func TestBlockRead2(t *testing.T) { info.MetaLocation().SetID(uint16(i)) b1 := buildBatch(colTyps) phyAddrColumnPos := -1 - cacheVectors := containers.NewVectors(len(colIdxs) + 1) + cacheVectors := containers.NewVectors(len(colIdxs) + 2) ds.SetTS(beforeDel) err = blockio.BlockDataReadInner( context.Background(), info, ds, colIdxs, colTyps, phyAddrColumnPos, diff --git a/pkg/vm/engine/tae/index/indexwrapper/mutindex.go b/pkg/vm/engine/tae/index/indexwrapper/mutindex.go index a091cc631a5e5..176166e418834 100644 --- a/pkg/vm/engine/tae/index/indexwrapper/mutindex.go +++ b/pkg/vm/engine/tae/index/indexwrapper/mutindex.go @@ -171,12 +171,6 @@ func (idx *MutIndex) GetDuplicatedRows( if err == index.ErrNotFound { return nil } - if skipFn != nil { - err = skipFn(rows[len(rows)-1]) - if err != nil { - return err - } - } var maxRow uint32 exist := false for i := len(rows) - 1; i >= 0; i-- { @@ -184,6 +178,15 @@ func (idx *MutIndex) GetDuplicatedRows( break } if int32(rows[i]) < maxVisibleRow { + if skipFn != nil { + err = skipFn(rows[i]) + if err == index.ErrNotFound { + continue + } + if err != nil { + return err + } + } maxRow = rows[i] exist = true break @@ -232,6 +235,9 @@ func (idx *MutIndex) Contains( panic("logic err: tombstones doesn't have duplicate rows") } err = skipFn(rows[0]) + if err == index.ErrNotFound { + return nil + } if err != nil { return err } diff --git a/pkg/vm/engine/tae/logtail/snapshot.go b/pkg/vm/engine/tae/logtail/snapshot.go index 351024fb2e042..be0c904e32989 100644 --- a/pkg/vm/engine/tae/logtail/snapshot.go +++ b/pkg/vm/engine/tae/logtail/snapshot.go @@ -588,10 +588,21 @@ func (sm *SnapshotMeta) updateTableInfo( if !obj.deleteAt.IsEmpty() { sm.aobjDelTsMap[obj.deleteAt] = struct{}{} } + location := obj.stats.ObjectLocation() + objectMeta, err := objectio.FastLoadObjectMeta(ctx, &location, false, fs) + if err != nil { + return err + } + blockMeta := objectMeta.MustDataMeta().GetBlockMeta(0) + specialLayout := objectio.ResolveSpecialColumnLayout(blockMeta) + commitPos, ok := specialLayout.Resolve(objectio.SEQNUM_COMMITTS) + if !ok { + return fmt.Errorf("snapshot table object has no commit timestamp") + } objectBat, _, err := ioutil.LoadOneBlock( ctx, fs, - obj.stats.ObjectLocation(), + location, objectio.SchemaData, ) if err != nil { @@ -600,7 +611,6 @@ func (sm *SnapshotMeta) updateTableInfo( // 0 is table id // 1 is table name // 11 is account id - // len(objectBat.Vecs)-1 is commit ts tids := vector.MustFixedColWithTypeCheck[uint64](objectBat.Vecs[0]) nameVarlena := vector.MustFixedColWithTypeCheck[types.Varlena](objectBat.Vecs[1]) nameArea := objectBat.Vecs[1].GetArea() @@ -608,8 +618,18 @@ func (sm *SnapshotMeta) updateTableInfo( dbArea := objectBat.Vecs[2].GetArea() dbs := vector.MustFixedColWithTypeCheck[uint64](objectBat.Vecs[3]) accounts := vector.MustFixedColWithTypeCheck[uint32](objectBat.Vecs[11]) - creates := vector.MustFixedColWithTypeCheck[types.TS](objectBat.Vecs[len(objectBat.Vecs)-1]) + creates := vector.MustFixedColWithTypeCheck[types.TS](objectBat.Vecs[commitPos]) + var aborts []bool + if abortPos, ok := specialLayout.Resolve(objectio.SEQNUM_ABORT); ok { + abortVec := objectBat.Vecs[abortPos] + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + } + } for i := 0; i < len(tids); i++ { + if aborts != nil && aborts[i] { + continue + } createAt := creates[i] if createAt.LT(&startts) || createAt.GT(&endts) { continue diff --git a/pkg/vm/engine/tae/logtail/tools.go b/pkg/vm/engine/tae/logtail/tools.go index 540d295fa0234..aabb1ebdd45f8 100644 --- a/pkg/vm/engine/tae/logtail/tools.go +++ b/pkg/vm/engine/tae/logtail/tools.go @@ -22,6 +22,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/common/mpool" "github.com/matrixorigin/matrixone/pkg/container/batch" "github.com/matrixorigin/matrixone/pkg/container/types" + "github.com/matrixorigin/matrixone/pkg/container/vector" "github.com/matrixorigin/matrixone/pkg/logutil" "github.com/matrixorigin/matrixone/pkg/objectio" "github.com/matrixorigin/matrixone/pkg/pb/api" @@ -113,21 +114,28 @@ func DataChangeToLogtailBatch(src *containers.BatchWithVersion) *containers.Batc panic("unmatched seqnums length") } + filterAbortedLogtailRows(src) + // sort by seqnum sort.Sort(src) bat := containers.NewBatchWithCapacity(int(src.NextSeqnum) + 2) - if src.Deletes != nil { - bat.Deletes = src.Deletes + rowIDPos, commitTSPos := -1, -1 + for i, seqnum := range src.Seqnums { + switch seqnum { + case objectio.SEQNUM_ROWID: + rowIDPos = i + case objectio.SEQNUM_COMMITTS: + commitTSPos = i + } } - - i := len(src.Seqnums) - 1 - // move special column first, no abort column in logtail - if src.Seqnums[i] != objectio.SEQNUM_ROWID || src.Seqnums[i-1] != objectio.SEQNUM_COMMITTS { - panic(fmt.Sprintf("bad last seqnums %v", src.Seqnums)) + // Abort is a storage-only column. Rolled-back rows were compacted above + // and are never emitted in logtail. + if rowIDPos == -1 || commitTSPos == -1 { + panic(fmt.Sprintf("missing required logtail seqnums in %v", src.Seqnums)) } - bat.AddVector(src.Attrs[i], src.Vecs[i]) - bat.AddVector(src.Attrs[i-1], src.Vecs[i-1]) + bat.AddVector(src.Attrs[rowIDPos], src.Vecs[rowIDPos]) + bat.AddVector(src.Attrs[commitTSPos], src.Vecs[commitTSPos]) for i, seqnum := range seqnums { if seqnum >= objectio.SEQNUM_UPPER { @@ -147,24 +155,47 @@ func TombstoneChangeToLogtailBatch(src *containers.BatchWithVersion) *containers if len(seqnums) != len(src.Vecs) { panic("unmatched seqnums length") } - if len(src.Vecs) != 4 { - panic(fmt.Sprintf("logic err, attr %v", src.Attrs)) - } + filterAbortedLogtailRows(src) bat := containers.NewBatchWithCapacity(3) - // move special column first, no abort column in logtail - if src.Seqnums[2] != objectio.SEQNUM_ROWID || src.Seqnums[3] != objectio.SEQNUM_COMMITTS { - panic(fmt.Sprintf("bad last seqnums %v", src.Seqnums)) + for _, attr := range []string{ + objectio.TombstoneAttr_Rowid_Attr, + objectio.TombstoneAttr_CommitTs_Attr, + objectio.TombstoneAttr_PK_Attr, + catalog.PhyAddrColumnName, + } { + vec := src.GetVectorByName(attr) + if vec == nil { + panic(fmt.Sprintf("missing tombstone logtail column %q in %v", attr, src.Attrs)) + } + bat.AddVector(attr, vec) } - bat.AddVector(src.Attrs[0], src.Vecs[0]) // rowid - bat.AddVector(src.Attrs[3], src.Vecs[3]) // committs - bat.AddVector(src.Attrs[1], src.Vecs[1]) // pk - bat.AddVector(src.Attrs[2], src.Vecs[2]) // PhyAddrColumn return bat } +func filterAbortedLogtailRows(src *containers.BatchWithVersion) { + abortPos := -1 + for i, seqnum := range src.Seqnums { + if seqnum == objectio.SEQNUM_ABORT { + abortPos = i + break + } + } + if abortPos != -1 && !src.Vecs[abortPos].IsConstNull() { + aborts := vector.MustFixedColWithTypeCheck[bool](src.Vecs[abortPos].GetDownstreamVector()) + for row, aborted := range aborts { + if aborted { + src.Delete(row) + } + } + } + if src.HasDelete() { + src.Compact() + } +} + func containersBatchToProtoBatch(bat *containers.Batch) (*api.Batch, error) { mobat := containers.ToCNBatch(bat) return batch.BatchToProtoBatch(mobat) diff --git a/pkg/vm/engine/tae/rpc/dump_table.go b/pkg/vm/engine/tae/rpc/dump_table.go index 37267e9c09172..400b5f4c809a7 100644 --- a/pkg/vm/engine/tae/rpc/dump_table.go +++ b/pkg/vm/engine/tae/rpc/dump_table.go @@ -377,6 +377,12 @@ func (c *DumpTableArg) onObject(e *catalog.ObjectEntry) error { if isPersisted, err = c.collectObjectList(e); err != nil { return err } + // Scan represents invisible append ranges (including rollback holes) in the + // batch delete bitmap. Dump-table objects do not carry appendable abort + // metadata, so compact those rows before handing the batch to restore. + if bat.HasDelete() { + bat.Compact() + } cnBatch := containers.ToCNBatch(bat) if isPersisted { name := e.ObjectStats.ObjectName().String() diff --git a/pkg/vm/engine/tae/rpc/tool.go b/pkg/vm/engine/tae/rpc/tool.go index 110ad61504a56..3602f54e714c1 100644 --- a/pkg/vm/engine/tae/rpc/tool.go +++ b/pkg/vm/engine/tae/rpc/tool.go @@ -775,6 +775,7 @@ func (c *objGetArg) GetData(ctx context.Context) (res string, err error) { } } + specialLayout := objectio.ResolveSpecialColumnLayout(blk) for _, i := range c.cols { idx := uint16(i) if idx >= cnt { @@ -783,9 +784,12 @@ func (c *objGetArg) GetData(ctx context.Context) (res string, err error) { } col := blk.ColumnMeta(idx) tp := types.T(col.DataType()).ToType() - if col.DataType() == uint8(types.T_TS) && i == len(c.cols)-1 { + switch idx { + case specialLayout.CommitTS: idxs = append(idxs, objectio.SEQNUM_COMMITTS) - } else { + case specialLayout.Abort: + idxs = append(idxs, objectio.SEQNUM_ABORT) + default: idxs = append(idxs, idx) } typs = append(typs, tp) diff --git a/pkg/vm/engine/tae/tables/base.go b/pkg/vm/engine/tae/tables/base.go index 3ceef4968b298..c8f2e99e19da2 100644 --- a/pkg/vm/engine/tae/tables/base.go +++ b/pkg/vm/engine/tae/tables/base.go @@ -164,10 +164,27 @@ func (obj *baseObject) LoadPersistedCommitTS(bid uint16) (vec containers.Vector, return obj.loadPersistedCommitTS(context.Background(), bid) } +func (obj *baseObject) LoadPersistedMVCC( + bid uint16, +) (commitTS, abort containers.Vector, err error) { + return obj.loadPersistedMVCC(context.Background(), bid) +} + func (obj *baseObject) loadPersistedCommitTS( ctx context.Context, bid uint16, ) (vec containers.Vector, err error) { + vec, abort, err := obj.loadPersistedMVCC(ctx, bid) + if abort != nil { + abort.Close() + } + return vec, err +} + +func (obj *baseObject) loadPersistedMVCC( + ctx context.Context, + bid uint16, +) (commitTS, abort containers.Vector, err error) { location, err := obj.buildMetalocation(bid) if err != nil { return @@ -181,8 +198,8 @@ func (obj *baseObject) loadPersistedCommitTS( // the caller owns and must close the returned vector. vectors, _, err := ioutil.LoadColumns2( ctx, - []uint16{objectio.SEQNUM_COMMITTS}, - []types.Type{types.T_TS.ToType()}, + []uint16{objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT}, + []types.Type{types.T_TS.ToType(), types.T_bool.ToType()}, obj.rt.Fs, location, fileservice.Policy(0), @@ -192,10 +209,22 @@ func (obj *baseObject) loadPersistedCommitTS( if err != nil { return } - return validatePersistedCommitTSVectors( - obj.meta.Load().ID().String(), - vectors, - ) + if len(vectors) != 2 || + vectors[0] == nil || vectors[0].GetType().Oid != types.T_TS || + vectors[1] == nil || vectors[1].GetType().Oid != types.T_bool || + vectors[0].Length() != vectors[1].Length() { + for _, vec := range vectors { + if vec != nil { + vec.Close() + } + } + err = moerr.NewInternalErrorNoCtxf( + "invalid persisted MVCC columns for object %s", + obj.meta.Load().ID().String(), + ) + return + } + return vectors[0], vectors[1], nil } func validatePersistedCommitTSVectors( @@ -294,8 +323,8 @@ func (obj *baseObject) getDuplicateRowsWithLoad( var dedupFn any if filterByCommitTS { loader := &commitTSLoader{ - load: func() (containers.Vector, error) { - return obj.loadPersistedCommitTS(ctx, blkOffset) + load: func() (containers.Vector, containers.Vector, error) { + return obj.loadPersistedMVCC(ctx, blkOffset) }, } defer loader.close() @@ -355,7 +384,7 @@ func (obj *baseObject) containsWithLoad( var dedupFn any if isAblk { dedupFn = containers.MakeForeachVectorOp( - keys.GetType().Oid, containsAlkFunctions, data.Vecs[0], keys, obj.LoadPersistedCommitTS, txn, + keys.GetType().Oid, containsAlkFunctions, data.Vecs[0], keys, obj.LoadPersistedMVCC, txn, func(vrowID any, commitTS types.TS) (types.TS, error) { rowID := vrowID.(types.Rowid) blkID := rowID.BorrowBlockID() diff --git a/pkg/vm/engine/tae/tables/functions.go b/pkg/vm/engine/tae/tables/functions.go index b7ee5da9e1bfd..c3a18e3468ef3 100644 --- a/pkg/vm/engine/tae/tables/functions.go +++ b/pkg/vm/engine/tae/tables/functions.go @@ -166,24 +166,36 @@ func parseAGetDuplicateRowIDsArgs(args ...any) ( // loaded for blocks without an actual candidate. Both success and failure are // cached to keep the storage read bounded to once per candidate block. type commitTSLoader struct { - load func() (containers.Vector, error) - vec containers.Vector - err error - loaded bool + load func() (containers.Vector, containers.Vector, error) + commitTS containers.Vector + abort containers.Vector + err error + loaded bool } -func (loader *commitTSLoader) get() (containers.Vector, error) { +func (loader *commitTSLoader) get() (containers.Vector, containers.Vector, error) { if !loader.loaded { - loader.vec, loader.err = loader.load() + loader.commitTS, loader.abort, loader.err = loader.load() loader.loaded = true } - return loader.vec, loader.err + return loader.commitTS, loader.abort, loader.err +} + +func isAborted(abort containers.Vector, row int) bool { + if abort == nil || abort.IsConstNull() || row >= abort.Length() { + return false + } + return vector.GetFixedAtNoTypeCheck[bool](abort.GetDownstreamVector(), row) } func (loader *commitTSLoader) close() { - if loader.vec != nil { - loader.vec.Close() - loader.vec = nil + if loader.commitTS != nil { + loader.commitTS.Close() + loader.commitTS = nil + } + if loader.abort != nil { + loader.abort.Close() + loader.abort = nil } } @@ -193,14 +205,15 @@ func missingCommitTS(vec containers.Vector, row int) bool { func parseAContainsArgs(args ...any) ( vec containers.Vector, rowIDs containers.Vector, - scanFn func(uint16) (vec containers.Vector, err error), txn txnif.TxnReader, delsFn func(rowID any, ts types.TS) (types.TS, error), + scanFn func(uint16) (commitTS, abort containers.Vector, err error), + txn txnif.TxnReader, delsFn func(rowID any, ts types.TS) (types.TS, error), ) { vec = args[0].(containers.Vector) if args[1] != nil { rowIDs = args[1].(containers.Vector) } if args[2] != nil { - scanFn = args[2].(func(bid uint16) (vec containers.Vector, err error)) + scanFn = args[2].(func(bid uint16) (commitTS, abort containers.Vector, err error)) } if args[3] != nil { txn = args[3].(txnif.TxnReader) @@ -314,10 +327,13 @@ func getDuplicatedRowIDABlkBytesFunc(args ...any) func([]byte, bool, int) error if compute.CompareBytes(v1, v2) != 0 { return } - tsVec, err := loader.get() + tsVec, abortVec, err := loader.get() if err != nil { return err } + if isAborted(abortVec, row) { + return nil + } // Legacy objects and TN rewrites containing CN-created rows may // not carry row commit timestamps. Incremental dedup is only a // fallback for lock/recheck paths, so it skips that unverifiable @@ -377,10 +393,13 @@ func getDuplicatedRowIDABlkFuncFactory[T types.FixedSizeT](comp func(T, T) int) if comp(v1, v2) != 0 { return } - tsVec, err := loader.get() + tsVec, abortVec, err := loader.get() if err != nil { return err } + if isAborted(abortVec, row) { + return nil + } // See the varlen path above: missing metadata is a // policy-aware compatibility fallback. if missingCommitTS(tsVec, row) { @@ -424,12 +443,16 @@ func containsABlkFuncFactory[T types.FixedSizeT](comp func(T, T) int) func(args if rowIDs.IsNull(rowOffset) { return nil } - var tsVec containers.Vector + var tsVec, abortVec containers.Vector defer func() { if tsVec != nil { tsVec.Close() tsVec = nil } + if abortVec != nil { + abortVec.Close() + abortVec = nil + } }() if row, existed := compute.GetOffsetWithFunc( vs, @@ -439,12 +462,15 @@ func containsABlkFuncFactory[T types.FixedSizeT](comp func(T, T) int) func(args ); existed { if tsVec == nil { var err error - tsVec, err = scanFn(0) + tsVec, abortVec, err = scanFn(0) if err != nil { return err } } rowIDs.Update(rowOffset, nil, true) + if isAborted(abortVec, row) { + return nil + } commitTS := tsVec.Get(row).(types.TS) startTS := txn.GetStartTS() if commitTS.GT(&startTS) { diff --git a/pkg/vm/engine/tae/tables/functions_test.go b/pkg/vm/engine/tae/tables/functions_test.go index ed344a5f6f917..54f7a88ae92cc 100644 --- a/pkg/vm/engine/tae/tables/functions_test.go +++ b/pkg/vm/engine/tae/tables/functions_test.go @@ -34,16 +34,17 @@ func TestCommitTSLoaderLoadsOnceAndReleases(t *testing.T) { loads := 0 loader := &commitTSLoader{ - load: func() (containers.Vector, error) { + load: func() (containers.Vector, containers.Vector, error) { loads++ - return commitTS, nil + return commitTS, nil, nil }, } for range 2 { - got, err := loader.get() + got, abort, err := loader.get() require.NoError(t, err) require.Same(t, commitTS, got) + require.Nil(t, abort) } require.Equal(t, 1, loads) loader.close() @@ -54,21 +55,60 @@ func TestCommitTSLoaderCachesError(t *testing.T) { loadErr := errors.New("load commit timestamps") loads := 0 loader := &commitTSLoader{ - load: func() (containers.Vector, error) { + load: func() (containers.Vector, containers.Vector, error) { loads++ - return nil, loadErr + return nil, nil, loadErr }, } for range 2 { - got, err := loader.get() + got, abort, err := loader.get() require.ErrorIs(t, err, loadErr) require.Nil(t, got) + require.Nil(t, abort) } require.Equal(t, 1, loads) loader.close() } +func TestPersistedAppendableDedupSkipsAbortedRow(t *testing.T) { + data := containers.MakeVector(types.T_int64.ToType(), common.DefaultAllocator) + defer data.Close() + data.Append(int64(42), false) + keys := containers.MakeVector(types.T_int64.ToType(), common.DefaultAllocator) + defer keys.Close() + keys.Append(int64(42), false) + rowIDs := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) + defer rowIDs.Close() + rowIDs.Append(nil, true) + + commitTS := containers.MakeVector(types.T_TS.ToType(), common.DefaultAllocator) + commitTS.Append(types.BuildTS(5, 0), false) + abort := containers.MakeVector(types.T_bool.ToType(), common.DefaultAllocator) + abort.Append(true, false) + loader := &commitTSLoader{ + load: func() (containers.Vector, containers.Vector, error) { + return commitTS, abort, nil + }, + } + defer loader.close() + + txn := txnbase.MockTxnReaderWithStartTS(types.BuildTS(10, 0)) + op := containers.MakeForeachVectorOp( + keys.GetType().Oid, + getRowIDAlkFunctions, + data, + rowIDs, + types.Blockid{}, + loader, + txn, + types.TS{}, + types.BuildTS(10, 0), + ) + require.NoError(t, containers.ForeachVector(keys, op, nil)) + require.True(t, rowIDs.IsNull(0)) +} + func TestMissingCommitTSFollowsDedupPolicy(t *testing.T) { for _, typ := range []types.Type{ types.T_int64.ToType(), @@ -142,8 +182,8 @@ func TestMissingCommitTSFollowsDedupPolicy(t *testing.T) { commitTS := commitTSCase.make() loader := &commitTSLoader{ - load: func() (containers.Vector, error) { - return commitTS, nil + load: func() (containers.Vector, containers.Vector, error) { + return commitTS, nil, nil }, } defer loader.close() @@ -204,8 +244,8 @@ func TestCommitTSAtDedupLowerBoundIsIncluded(t *testing.T) { ) commitTS.Append(from, false) loader := &commitTSLoader{ - load: func() (containers.Vector, error) { - return commitTS, nil + load: func() (containers.Vector, containers.Vector, error) { + return commitTS, nil, nil }, } defer loader.close() @@ -257,9 +297,9 @@ func TestCommitTSIsNotLoadedWithoutExactPKMatch(t *testing.T) { loads := 0 loader := &commitTSLoader{ - load: func() (containers.Vector, error) { + load: func() (containers.Vector, containers.Vector, error) { loads++ - return nil, errors.New("unexpected commit-TS read") + return nil, nil, errors.New("unexpected commit-TS read") }, } defer loader.close() diff --git a/pkg/vm/engine/tae/tables/jobs/flushTableTail.go b/pkg/vm/engine/tae/tables/jobs/flushTableTail.go index b07ebf6b50bd5..79f06bda7873d 100644 --- a/pkg/vm/engine/tae/tables/jobs/flushTableTail.go +++ b/pkg/vm/engine/tae/tables/jobs/flushTableTail.go @@ -89,6 +89,7 @@ type flushTableTailTask struct { createdMergedTombstoneName string mergeRowsCnt, aObjDeletesCnt, tombstoneMergeRowsCnt int + aTombstoneDeletesCnt int createAt time.Time } @@ -519,10 +520,10 @@ func (task *flushTableTailTask) prepareAObjSortedData( } } totalRowCnt := bat.Length() - task.aObjDeletesCnt += bat.Deletes.GetCardinality() - - if isTombstone && bat.Deletes.GetCardinality() > 0 { - panic(fmt.Sprintf("logic err, tombstone %v has deletes", obj.GetID().String())) + if isTombstone { + task.aTombstoneDeletesCnt += bat.Deletes.GetCardinality() + } else { + task.aObjDeletesCnt += bat.Deletes.GetCardinality() } var sortMapping []int64 @@ -633,7 +634,9 @@ func (task *flushTableTailTask) mergeAObjs(ctx context.Context, isTombstone bool vec := bat.Vecs[sortKeyPos] totalRowCnt += vec.Length() } - if !isTombstone { + if isTombstone { + totalRowCnt -= task.aTombstoneDeletesCnt + } else { totalRowCnt -= task.aObjDeletesCnt } if isTombstone { diff --git a/pkg/vm/engine/tae/tables/mnode.go b/pkg/vm/engine/tae/tables/mnode.go index 7a658f622aae2..b7d4c720bc055 100644 --- a/pkg/vm/engine/tae/tables/mnode.go +++ b/pkg/vm/engine/tae/tables/mnode.go @@ -158,9 +158,6 @@ func (node *memoryNode) getDataWindowOnWriteSchema( } from, to, commitTSVec, abort, _ := node.object.appendMVCC.CollectAppendLocked(start, end, mp) - if abort != nil { - abort.Close() - } if commitTSVec == nil { return nil } @@ -168,7 +165,9 @@ func (node *memoryNode) getDataWindowOnWriteSchema( if ok { dest.Extend(node.data.Window(int(from), int(to-from))) dest.GetVectorByName(objectio.TombstoneAttr_CommitTs_Attr).Extend(commitTSVec) + dest.GetVectorByName(objectio.TombstoneAttr_Abort_Attr).Extend(abort) commitTSVec.Close() // TODO no copy + abort.Close() } else { inner := node.data.CloneWindowWithPool(int(from), int(to-from), node.object.rt.VectorPool.Transient) batWithVer := &containers.BatchWithVersion{ @@ -178,7 +177,9 @@ func (node *memoryNode) getDataWindowOnWriteSchema( Batch: inner, } inner.AddVector(objectio.TombstoneAttr_CommitTs_Attr, commitTSVec) + inner.AddVector(objectio.TombstoneAttr_Abort_Attr, abort) batWithVer.Seqnums = append(batWithVer.Seqnums, objectio.SEQNUM_COMMITTS) + batWithVer.Seqnums = append(batWithVer.Seqnums, objectio.SEQNUM_ABORT) batches[node.writeSchema.Version] = batWithVer } return @@ -208,6 +209,12 @@ func (node *memoryNode) getDataWindowLocked( (*bat).AddVector(objectio.TombstoneAttr_CommitTs_Attr, vec) continue } + if colIdx == objectio.SEQNUM_ABORT { + typ := types.T_bool.ToType() + vec := node.object.rt.VectorPool.Transient.GetVector(&typ) + (*bat).AddVector(objectio.TombstoneAttr_Abort_Attr, vec) + continue + } colDef := readSchema.ColDefs[colIdx] idx, ok := node.writeSchema.SeqnumMap[colDef.SeqNum] var vec containers.Vector @@ -223,7 +230,7 @@ func (node *memoryNode) getDataWindowLocked( } } else { for _, colIdx := range colIdxes { - if colIdx == objectio.SEQNUM_COMMITTS { + if colIdx == objectio.SEQNUM_COMMITTS || colIdx == objectio.SEQNUM_ABORT { continue } colDef := readSchema.ColDefs[colIdx] @@ -293,6 +300,9 @@ func (node *memoryNode) checkConflictLocked( ) func(row uint32) error { return func(row uint32) error { appendnode := node.object.appendMVCC.GetAppendNodeByRowLocked(row) + if appendnode.IsAborted() { + return index.ErrNotFound + } // Deletes generated by merge/flush is ignored when check w-w in batchDedup if appendnode.IsMergeCompact() { return nil @@ -324,11 +334,15 @@ func (node *memoryNode) Scan( } node.object.RLock() defer node.object.RUnlock() - maxRow, visible, _, err := node.object.appendMVCC.GetVisibleRowLocked(ctx, txn) + maxRow, visible, holes, err := node.object.appendMVCC.GetVisibleRowLocked(ctx, txn) if !visible || err != nil { // blk.RUnlock() return } + rowOffset := 0 + if *bat != nil { + rowOffset = (*bat).Length() + } err = node.getDataWindowLocked( bat, readSchema, @@ -343,6 +357,12 @@ func (node *memoryNode) Scan( (*bat).GetVectorByName(objectio.TombstoneAttr_CommitTs_Attr), maxRow, mp) } } + if !holes.IsEmpty() { + holes.Foreach(func(row uint64) bool { + (*bat).Delete(rowOffset + int(row)) + return true + }) + } return } @@ -366,8 +386,12 @@ func (node *memoryNode) CollectObjectTombstoneInRange( rowIDs := vector.MustFixedColWithTypeCheck[types.Rowid]( node.data.GetVectorByName(objectio.TombstoneAttr_Rowid_Attr).GetDownstreamVector()) commitTSs := vector.MustFixedColWithTypeCheck[types.TS](commitTSVec.GetDownstreamVector()) + aborts := vector.MustFixedColWithTypeCheck[bool](abort.GetDownstreamVector()) pkVec := node.data.GetVectorByName(objectio.TombstoneAttr_PK_Attr) for i := minRow; i < maxRow; i++ { + if aborts[i-minRow] { + continue + } if types.PrefixCompare(rowIDs[i][:], objID[:]) == 0 { if *bat == nil { *bat = catalog.NewTombstoneBatchByPKType(*pkVec.GetType(), mp) @@ -389,7 +413,7 @@ func (node *memoryNode) FillBlockTombstones( mp *mpool.MPool) error { node.object.RLock() defer node.object.RUnlock() - maxRow, visible, _, err := node.object.appendMVCC.GetVisibleRowLocked(ctx, txn) + maxRow, visible, holes, err := node.object.appendMVCC.GetVisibleRowLocked(ctx, txn) if !visible || err != nil { // blk.RUnlock() return err @@ -397,6 +421,9 @@ func (node *memoryNode) FillBlockTombstones( rowIDVec := node.data.GetVectorByName(objectio.TombstoneAttr_Rowid_Attr) rowIDs := vector.MustFixedColWithTypeCheck[types.Rowid](rowIDVec.GetDownstreamVector()) for i := 0; i < int(maxRow); i++ { + if holes.Contains(uint64(i)) { + continue + } rowID := rowIDs[i] if types.PrefixCompare(rowID[:], blkID[:]) == 0 { if *deletes == nil { diff --git a/pkg/vm/engine/tae/tables/object_test.go b/pkg/vm/engine/tae/tables/object_test.go index e9a5664e7dab2..ce1b733016081 100644 --- a/pkg/vm/engine/tae/tables/object_test.go +++ b/pkg/vm/engine/tae/tables/object_test.go @@ -15,15 +15,18 @@ package tables import ( + "context" "testing" "github.com/matrixorigin/matrixone/pkg/common/mpool" "github.com/matrixorigin/matrixone/pkg/container/types" + "github.com/matrixorigin/matrixone/pkg/container/vector" "github.com/matrixorigin/matrixone/pkg/objectio" api "github.com/matrixorigin/matrixone/pkg/pb/api" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/catalog" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/common" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/db/dbutils" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/index/indexwrapper" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/tables/updates" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/testutils" @@ -194,3 +197,84 @@ func TestApplyAppendLockedPadsMissingColumnsForUpgradedSchema(t *testing.T) { win.Close() }) } + +func TestMemoryNodeScanMarksRollbackHoleDeleted(t *testing.T) { + defer testutils.AfterTest(t)() + schema := catalog.MockSchema(1, 0) + c := catalog.MockCatalog(nil) + defer c.Close() + db, _ := c.CreateDBEntry("db", "", "", nil) + table, _ := db.CreateTableEntry(schema, nil, nil) + noid := objectio.NewObjectid() + stats := objectio.NewObjectStatsWithObjectID(&noid, true, false, false) + obj, _ := table.CreateObject(nil, &objectio.CreateObjOpt{Stats: stats}, nil) + + mvcc := updates.NewAppendMVCCHandle(obj) + base := &baseObject{ + RWMutex: mvcc.RWMutex, + appendMVCC: mvcc, + rt: dbutils.NewRuntime(), + } + base.meta.Store(obj) + mnode := &memoryNode{object: base, writeSchema: schema} + + input := containers.BuildBatch( + schema.AllNames(), + schema.AllTypes(), + containers.Options{Allocator: common.DefaultAllocator}, + ) + defer input.Close() + for _, vec := range input.Vecs { + for range 3 { + vec.Append(nil, true) + } + } + _, err := mnode.ApplyAppendLocked(input) + require.NoError(t, err) + defer mnode.data.Close() + + for row := uint32(0); row < 3; row++ { + appendNode, _ := mvcc.AddAppendNodeLocked(nil, row, row+1) + ts := types.BuildTS(int64(row+1), 0) + appendNode.Start, appendNode.Prepare, appendNode.End = ts, ts, ts + if row == 1 { + appendNode.Aborted = true + } + } + + flushBatches := make(map[uint32]*containers.BatchWithVersion) + err = mnode.getDataWindowOnWriteSchema( + context.Background(), + flushBatches, + types.TS{}, + types.BuildTS(3, 0), + common.DefaultAllocator, + ) + require.NoError(t, err) + flushBatch := flushBatches[schema.Version] + require.NotNil(t, flushBatch) + defer flushBatch.Close() + require.Contains(t, flushBatch.Seqnums, uint16(objectio.SEQNUM_ABORT)) + aborts := vector.MustFixedColWithTypeCheck[bool]( + flushBatch.GetVectorByName(objectio.TombstoneAttr_Abort_Attr).GetDownstreamVector(), + ) + require.Equal(t, []bool{false, true, false}, aborts) + + var output *containers.Batch + err = mnode.Scan( + context.Background(), + &output, + updates.MockTxnWithStartTS(types.BuildTS(3, 0)), + schema, + 0, + []int{0}, + common.DefaultAllocator, + ) + require.NoError(t, err) + require.NotNil(t, output) + defer output.Close() + require.Equal(t, 3, output.Length()) + require.True(t, output.IsDeleted(1)) + require.False(t, output.IsDeleted(0)) + require.False(t, output.IsDeleted(2)) +} diff --git a/pkg/vm/engine/tae/tables/pnode.go b/pkg/vm/engine/tae/tables/pnode.go index b8c872a196810..28f3942b0be49 100644 --- a/pkg/vm/engine/tae/tables/pnode.go +++ b/pkg/vm/engine/tae/tables/pnode.go @@ -16,6 +16,7 @@ package tables import ( "context" + "slices" pkgcatalog "github.com/matrixorigin/matrixone/pkg/catalog" "github.com/matrixorigin/matrixone/pkg/common/mpool" @@ -141,6 +142,8 @@ func (node *persistedNode) Scan( replaceCommitts(vecs, i) } /// TODO: Read old version of nonappendable block? + } else if idx == objectio.SEQNUM_ABORT { + attr = objectio.TombstoneAttr_Abort_Attr } else { attr = readSchema.ColDefs[idx].Name } @@ -175,6 +178,8 @@ func (node *persistedNode) Scan( if vecs[i].IsConstNull() { replaceCommitts(vecs, i) } + } else if idx == objectio.SEQNUM_ABORT { + attr = objectio.TombstoneAttr_Abort_Attr } else { attr = readSchema.ColDefs[idx].Name } @@ -233,7 +238,10 @@ func (node *persistedNode) CollectObjectTombstoneInRange( if !node.object.meta.Load().IsTombstone { panic("not support") } - colIdxes := objectio.TombstoneColumns_TN_Created + colIdxes := append( + slices.Clone(objectio.TombstoneColumns_TN_Created), + objectio.SEQNUM_ABORT, + ) readSchema := node.object.meta.Load().GetTable().GetLastestSchema(true) var startTS types.TS if !node.object.meta.Load().IsAppendable() { @@ -264,8 +272,9 @@ func (node *persistedNode) CollectObjectTombstoneInRange( if err != nil { return err } - colCount := objDataMeta.MustGetMeta(objectio.SchemaData).GetBlockMeta(0).GetColumnCount() - persistedByCN := colCount == 2 + firstBlock := objDataMeta.MustGetMeta(objectio.SchemaData).GetBlockMeta(0) + persistedByCN := objectio.ResolveSpecialColumnLayout(firstBlock).CommitTS == + objectio.InvalidSpecialColumnPosition for blkID := 0; blkID < node.object.meta.Load().BlockCnt(); blkID++ { buf := bf.GetBloomFilter(uint32(blkID)) bfIndex := index.NewEmptyBloomFilterWithType(index.HBF) @@ -294,8 +303,16 @@ func (node *persistedNode) CollectObjectTombstoneInRange( var commitTSs []types.TS if !persistedByCN { commitTSs = vector.MustFixedColWithTypeCheck[types.TS](vecs[2].GetDownstreamVector()) + abortVec := vecs[3] + var aborts []bool + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec.GetDownstreamVector()) + } rowIDs := vector.MustFixedColWithTypeCheck[types.Rowid](vecs[0].GetDownstreamVector()) for i := 0; i < len(commitTSs); i++ { + if aborts != nil && aborts[i] { + continue + } commitTS := commitTSs[i] if commitTS.GE(&start) && commitTS.LE(&end) && types.PrefixCompare(rowIDs[i][:], objID[:]) == 0 { // TODO @@ -360,6 +377,11 @@ func (node *persistedNode) FillBlockTombstones( return err } colIdxs := []int{0} + var snapshotTS *types.TS + if node.object.meta.Load().IsAppendable() { + colIdxs = append(colIdxs, objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT) + snapshotTS = &startTS + } for tombstoneBlkID := 0; tombstoneBlkID < node.object.meta.Load().BlockCnt(); tombstoneBlkID++ { buf := bf.GetBloomFilter(uint32(tombstoneBlkID)) bfIndex := index.NewEmptyBloomFilterWithType(index.HBF) @@ -378,32 +400,17 @@ func (node *persistedNode) FillBlockTombstones( if err != nil { return err } - vecs, _, _, err := LoadPersistedColumnData( - ctx, readSchema, node.object.rt, id, colIdxs, location, mp, nil, + vecs, visibilityDeletes, _, err := LoadPersistedColumnData( + ctx, readSchema, node.object.rt, id, colIdxs, location, mp, snapshotTS, true, ) if err != nil { return err } - var commitTSs []types.TS - var commitTSVec containers.Vector - if node.object.meta.Load().IsAppendable() { - commitTSVec, err = node.object.LoadPersistedCommitTS(uint16(tombstoneBlkID)) - if err != nil { - for i := range vecs { - vecs[i].Close() - } - return err - } - commitTSs = vector.MustFixedColWithTypeCheck[types.TS](commitTSVec.GetDownstreamVector()) - } rowIDs := vector.MustFixedColWithTypeCheck[types.Rowid](vecs[0].GetDownstreamVector()) - // TODO: biselect, check visibility for i := 0; i < len(rowIDs); i++ { - if node.object.meta.Load().IsAppendable() { - if commitTSs[i].GT(&startTS) { - continue - } + if visibilityDeletes != nil && visibilityDeletes.Contains(uint64(i)) { + continue } rowID := rowIDs[i] if types.PrefixCompare(rowID[:], blkID[:]) == 0 { @@ -414,9 +421,6 @@ func (node *persistedNode) FillBlockTombstones( (*deletes).Add(uint64(offset) + deleteStartOffset) } } - if commitTSVec != nil { - commitTSVec.Close() - } for i := range vecs { vecs[i].Close() } diff --git a/pkg/vm/engine/tae/tables/updates/append.go b/pkg/vm/engine/tae/tables/updates/append.go index 6901fef455067..d7a15252b02e4 100644 --- a/pkg/vm/engine/tae/tables/updates/append.go +++ b/pkg/vm/engine/tae/tables/updates/append.go @@ -154,6 +154,12 @@ func (node *AppendNode) ApplyRollback() (err error) { } func (node *AppendNode) WriteTo(w io.Writer) (n int64, err error) { + return node.WriteToVersion(w, IOET_WALTxnCommand_AppendNode_CurrVer) +} + +func (node *AppendNode) WriteToVersion( + w io.Writer, version uint16, +) (n int64, err error) { cn, err := w.Write(common.EncodeID(node.mvcc.GetID())) if err != nil { return @@ -178,10 +184,22 @@ func (node *AppendNode) WriteTo(w io.Writer) (n int64, err error) { return } n += int64(sn1) + if version >= IOET_WALTxnCommand_AppendNode_V2 { + if sn1, err = w.Write(types.EncodeBool(&node.Aborted)); err != nil { + return + } + n += int64(sn1) + } return } func (node *AppendNode) ReadFrom(r io.Reader) (n int64, err error) { + return node.ReadFromVersion(r, IOET_WALTxnCommand_AppendNode_CurrVer) +} + +func (node *AppendNode) ReadFromVersion( + r io.Reader, version uint16, +) (n int64, err error) { var sn int if node.id == nil { node.id = &common.ID{} @@ -208,14 +226,25 @@ func (node *AppendNode) ReadFrom(r io.Reader) (n int64, err error) { return } n += int64(sn) + if version >= IOET_WALTxnCommand_AppendNode_V2 { + if sn, err = r.Read(types.EncodeBool(&node.Aborted)); err != nil { + return + } + n += int64(sn) + } else { + node.Aborted = false + } return } func (node *AppendNode) PrepareRollback() (err error) { node.mvcc.Lock() defer node.mvcc.Unlock() - node.mvcc.DeleteAppendNodeLocked(node) - return + // Append data and its PK index entry are installed before commit. Keep the + // MVCC owner for that physical range and mark it aborted; removing the node + // would leave the rows ownerless and makes a later flush unable to persist + // the rollback hole. + return node.TxnMVCCNode.PrepareRollback() } func (node *AppendNode) MakeCommand(id uint32) (cmd txnif.TxnCmd, err error) { cmd = NewAppendCmd(id, node) diff --git a/pkg/vm/engine/tae/tables/updates/cmd.go b/pkg/vm/engine/tae/tables/updates/cmd.go index 780326b7ba991..14dcd167ee07d 100644 --- a/pkg/vm/engine/tae/tables/updates/cmd.go +++ b/pkg/vm/engine/tae/tables/updates/cmd.go @@ -33,24 +33,30 @@ const ( IOET_WALTxnCommand_PersistedDeleteNode uint16 = 3013 IOET_WALTxnCommand_AppendNode_V1 uint16 = 1 + IOET_WALTxnCommand_AppendNode_V2 uint16 = 2 - IOET_WALTxnCommand_AppendNode_CurrVer = IOET_WALTxnCommand_AppendNode_V1 + IOET_WALTxnCommand_AppendNode_CurrVer = IOET_WALTxnCommand_AppendNode_V2 ) func init() { - objectio.RegisterIOEnrtyCodec( - objectio.IOEntryHeader{ - Type: IOET_WALTxnCommand_AppendNode, - Version: IOET_WALTxnCommand_AppendNode_V1, - }, - nil, - func(b []byte) (any, error) { - txnCmd := NewEmptyCmd(IOET_WALTxnCommand_AppendNode, - IOET_WALTxnCommand_AppendNode_V1) - err := txnCmd.UnmarshalBinary(b) - return txnCmd, err - }, - ) + for _, version := range []uint16{ + IOET_WALTxnCommand_AppendNode_V1, + IOET_WALTxnCommand_AppendNode_V2, + } { + version := version + objectio.RegisterIOEnrtyCodec( + objectio.IOEntryHeader{ + Type: IOET_WALTxnCommand_AppendNode, + Version: version, + }, + nil, + func(b []byte) (any, error) { + txnCmd := NewEmptyCmd(IOET_WALTxnCommand_AppendNode, version) + err := txnCmd.UnmarshalBinary(b) + return txnCmd, err + }, + ) + } } type UpdateCmd struct { @@ -58,12 +64,14 @@ type UpdateCmd struct { dest *common.ID append *AppendNode cmdType uint16 + version uint16 } func NewEmptyCmd(cmdType uint16, version uint16) *UpdateCmd { cmd := &UpdateCmd{} cmd.BaseCustomizedCmd = txnbase.NewBaseCustomizedCmd(0, cmd) cmd.cmdType = cmdType + cmd.version = version if cmdType == IOET_WALTxnCommand_AppendNode { cmd.append = NewAppendNode(nil, 0, 0, false, nil) } else { @@ -76,6 +84,7 @@ func NewAppendCmd(id uint32, app *AppendNode) *UpdateCmd { impl := &UpdateCmd{ append: app, cmdType: IOET_WALTxnCommand_AppendNode, + version: IOET_WALTxnCommand_AppendNode_CurrVer, dest: app.mvcc.meta.AsCommonID(), } impl.BaseCustomizedCmd = txnbase.NewBaseCustomizedCmd(id, impl) @@ -100,6 +109,9 @@ func (c *UpdateCmd) SetReplayTxn(txn txnif.AsyncTxn) { func (c *UpdateCmd) GetCurrentVersion() uint16 { switch c.cmdType { case IOET_WALTxnCommand_AppendNode: + if c.version != 0 { + return c.version + } return IOET_WALTxnCommand_AppendNode_CurrVer default: panic(fmt.Sprintf("invalid command type %d", c.cmdType)) @@ -182,7 +194,7 @@ func (c *UpdateCmd) WriteTo(w io.Writer) (n int64, err error) { n += common.IDSize switch c.GetType() { case IOET_WALTxnCommand_AppendNode: - sn, err = c.append.WriteTo(w) + sn, err = c.append.WriteToVersion(w, ver) } n += sn return @@ -198,7 +210,7 @@ func (c *UpdateCmd) ReadFrom(r io.Reader) (n int64, err error) { } switch c.GetType() { case IOET_WALTxnCommand_AppendNode: - n, err = c.append.ReadFrom(r) + n, err = c.append.ReadFromVersion(r, c.version) } n += 4 + common.IDSize return diff --git a/pkg/vm/engine/tae/tables/updates/command_test.go b/pkg/vm/engine/tae/tables/updates/command_test.go index fe6e2b73f2988..c4cfb1532578b 100644 --- a/pkg/vm/engine/tae/tables/updates/command_test.go +++ b/pkg/vm/engine/tae/tables/updates/command_test.go @@ -45,6 +45,7 @@ func TestCompactBlockCmd(t *testing.T) { ts := types.NextGlobalTsForTest() //node := MockAppendNode(341, 0, 2515, controller) node := MockAppendNode(ts, 0, 2515, controller) + node.Aborted = true cmd := NewAppendCmd(1, node) buf, err := cmd.MarshalBinary() @@ -59,6 +60,31 @@ func checkAppendCmdIsEqual(t *testing.T, cmd1, cmd2 *UpdateCmd) { assert.Equal(t, IOET_WALTxnCommand_AppendNode, cmd1.GetType()) assert.Equal(t, IOET_WALTxnCommand_AppendNode, cmd2.GetType()) assert.Equal(t, cmd1.append.maxRow, cmd2.append.maxRow) + assert.Equal(t, cmd1.append.Aborted, cmd2.append.Aborted) +} + +func TestAppendCmdV1CompatibilityDefaultsToNotAborted(t *testing.T) { + defer testutils.AfterTest(t)() + schema := catalog.MockSchema(1, 0) + c := catalog.MockCatalog(nil) + defer c.Close() + + db, _ := c.CreateDBEntry("db", "", "", nil) + table, _ := db.CreateTableEntry(schema, nil, nil) + noid := objectio.NewObjectid() + stats := objectio.NewObjectStatsWithObjectID(&noid, true, false, false) + obj, _ := table.CreateObject(nil, &objectio.CreateObjOpt{Stats: stats}, nil) + + controller := NewAppendMVCCHandle(obj) + node := MockAppendNode(types.NextGlobalTsForTest(), 0, 10, controller) + cmd := NewAppendCmd(1, node) + cmd.version = IOET_WALTxnCommand_AppendNode_V1 + + buf, err := cmd.MarshalBinary() + assert.NoError(t, err) + decoded, err := txnbase.BuildCommandFrom(buf) + assert.NoError(t, err) + assert.False(t, decoded.(*UpdateCmd).append.Aborted) } // errorUpdateCmd is an UpdateCmd that returns error on MarshalBinaryWithBuffer diff --git a/pkg/vm/engine/tae/tables/updates/mvcc.go b/pkg/vm/engine/tae/tables/updates/mvcc.go index 749f2a0810d2b..4456610391e8d 100644 --- a/pkg/vm/engine/tae/tables/updates/mvcc.go +++ b/pkg/vm/engine/tae/tables/updates/mvcc.go @@ -340,12 +340,6 @@ func (n *AppendMVCCHandle) allAppendsCommittedLocked() bool { return n.appends.IsCommitted() } -// DeleteAppendNodeLocked deletes the appendnode from the append list. -// it is called when txn of the appendnode is aborted. -func (n *AppendMVCCHandle) DeleteAppendNodeLocked(node *AppendNode) { - n.appends.DeleteNode(node) -} - func (n *AppendMVCCHandle) SetAppendListener(l func(txnif.AppendNode) error) { n.appendListener = l } diff --git a/pkg/vm/engine/tae/tables/updates/mvcc_test.go b/pkg/vm/engine/tae/tables/updates/mvcc_test.go index 14cb75ea82e60..401f4bda71c24 100644 --- a/pkg/vm/engine/tae/tables/updates/mvcc_test.go +++ b/pkg/vm/engine/tae/tables/updates/mvcc_test.go @@ -25,6 +25,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/catalog" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/testutils" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMutationControllerAppend(t *testing.T) { @@ -137,3 +138,22 @@ func TestGetVisibleRow(t *testing.T) { assert.Equal(t, 0, holes.GetCardinality()) } + +func TestPrepareRollbackKeepsAppendRangeAsHole(t *testing.T) { + defer testutils.AfterTest(t)() + schema := catalog.MockSchema(1, 0) + c := catalog.MockCatalog(nil) + defer c.Close() + db, _ := c.CreateDBEntry("db", "", "", nil) + table, _ := db.CreateTableEntry(schema, nil, nil) + noid := objectio.NewObjectid() + stats := objectio.NewObjectStatsWithObjectID(&noid, true, false, false) + obj, _ := table.CreateObject(nil, &objectio.CreateObjOpt{Stats: stats}, nil) + handle := NewAppendMVCCHandle(obj) + + node, _ := handle.AddAppendNodeLocked(nil, 2, 5) + require.NoError(t, node.PrepareRollback()) + require.True(t, node.IsAborted()) + require.Same(t, node, handle.GetAppendNodeByRowLocked(3)) + require.Equal(t, uint32(5), handle.GetTotalRow()) +} diff --git a/pkg/vm/engine/tae/tables/utils.go b/pkg/vm/engine/tae/tables/utils.go index 150b84f5199be..3284dd6ddba4a 100644 --- a/pkg/vm/engine/tae/tables/utils.go +++ b/pkg/vm/engine/tae/tables/utils.go @@ -60,45 +60,60 @@ func LoadPersistedColumnData( cols := make([]uint16, 0, len(colIdxs)) typs := make([]types.Type, 0, len(colIdxs)) vectors := make([]containers.Vector, len(colIdxs)) - phyAddIdx := -1 - committsIdx := -1 - assignedCommitts := false + outputPositions := make([]int, 0, len(colIdxs)) + commitTSIdx := -1 + abortIdx := -1 var deletes *nulls.Nulls for i, colIdx := range colIdxs { - if colIdx == objectio.SEQNUM_COMMITTS { + switch colIdx { + case objectio.SEQNUM_COMMITTS: cols = append(cols, objectio.SEQNUM_COMMITTS) typs = append(typs, objectio.TSType) - committsIdx = len(cols) - 1 - assignedCommitts = true + outputPositions = append(outputPositions, i) + commitTSIdx = len(cols) - 1 + continue + case objectio.SEQNUM_ABORT: + cols = append(cols, objectio.SEQNUM_ABORT) + typs = append(typs, types.T_bool.ToType()) + outputPositions = append(outputPositions, i) + abortIdx = len(cols) - 1 continue } def := schema.ColDefs[colIdx] if def.IsPhyAddr() { vec, err := PreparePhyAddrData(&id.BlockID, 0, location.Rows(), rt.VectorPool.Transient) if err != nil { + for _, existing := range vectors { + if existing != nil { + existing.Close() + } + } return nil, deletes, nil, err } - phyAddIdx = i - vectors[phyAddIdx] = vec + vectors[i] = vec continue } cols = append(cols, def.SeqNum) typs = append(typs, def.Type) - } - if len(cols) == 0 { - return vectors, deletes, nil, nil + outputPositions = append(outputPositions, i) } if tsForAppendable != nil { - if committsIdx == -1 { + if commitTSIdx == -1 { cols = append(cols, objectio.SEQNUM_COMMITTS) typs = append(typs, types.T_TS.ToType()) - committsIdx = len(cols) - 1 - defer func() { - cols = cols[:len(cols)-1] - typs = typs[:len(typs)-1] - }() + outputPositions = append(outputPositions, -1) + commitTSIdx = len(cols) - 1 + } + if abortIdx == -1 { + cols = append(cols, objectio.SEQNUM_ABORT) + typs = append(typs, types.T_bool.ToType()) + outputPositions = append(outputPositions, -1) + abortIdx = len(cols) - 1 } } + if len(cols) == 0 { + return vectors, deletes, nil, nil + } var vecs []containers.Vector var release func() var err error @@ -111,29 +126,36 @@ func LoadPersistedColumnData( needCopy, rt.VectorPool.Transient) if err != nil { + for _, vec := range vectors { + if vec != nil { + vec.Close() + } + } return nil, deletes, nil, err } if tsForAppendable != nil { - commits := vector.MustFixedColNoTypeCheck[types.TS](vecs[committsIdx].GetDownstreamVector()) + commits := vector.MustFixedColNoTypeCheck[types.TS](vecs[commitTSIdx].GetDownstreamVector()) + abortVec := vecs[abortIdx] + var aborts []bool + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColNoTypeCheck[bool](abortVec.GetDownstreamVector()) + } for i := range commits { - if commits[i].GT(tsForAppendable) { + if commits[i].GT(tsForAppendable) || (aborts != nil && aborts[i]) { if deletes == nil { deletes = nulls.NewWithSize(int(location.Rows())) } deletes.Add(uint64(i)) } } - if !assignedCommitts { - vecs[committsIdx].Close() - vecs = vecs[:len(vecs)-1] - } } for i, vec := range vecs { - idx := i - if idx >= phyAddIdx && phyAddIdx > -1 { - idx++ + outputPos := outputPositions[i] + if outputPos == -1 { + vec.Close() + continue } - vectors[idx] = vec + vectors[outputPos] = vec } return vectors, deletes, release, nil } From 70e953fedaa6cd153869f579e32668bba0e57f8f Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Wed, 29 Jul 2026 10:33:56 +0800 Subject: [PATCH 02/11] fix(logtail): use moerr for snapshot metadata error --- pkg/vm/engine/tae/logtail/snapshot.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/vm/engine/tae/logtail/snapshot.go b/pkg/vm/engine/tae/logtail/snapshot.go index be0c904e32989..58a031f9966cf 100644 --- a/pkg/vm/engine/tae/logtail/snapshot.go +++ b/pkg/vm/engine/tae/logtail/snapshot.go @@ -25,6 +25,7 @@ import ( "go.uber.org/zap" catalog2 "github.com/matrixorigin/matrixone/pkg/catalog" + "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/common/mpool" "github.com/matrixorigin/matrixone/pkg/common/util" "github.com/matrixorigin/matrixone/pkg/container/batch" @@ -597,7 +598,7 @@ func (sm *SnapshotMeta) updateTableInfo( specialLayout := objectio.ResolveSpecialColumnLayout(blockMeta) commitPos, ok := specialLayout.Resolve(objectio.SEQNUM_COMMITTS) if !ok { - return fmt.Errorf("snapshot table object has no commit timestamp") + return moerr.NewInternalError(ctx, "snapshot table object has no commit timestamp") } objectBat, _, err := ioutil.LoadOneBlock( ctx, From 95241243e728660b0871fc9e686554e069beec62 Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Wed, 29 Jul 2026 11:26:46 +0800 Subject: [PATCH 03/11] fix(tae): remove redundant loop variable copy --- pkg/vm/engine/tae/tables/updates/cmd.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/vm/engine/tae/tables/updates/cmd.go b/pkg/vm/engine/tae/tables/updates/cmd.go index 14dcd167ee07d..51c534d6e230b 100644 --- a/pkg/vm/engine/tae/tables/updates/cmd.go +++ b/pkg/vm/engine/tae/tables/updates/cmd.go @@ -43,7 +43,6 @@ func init() { IOET_WALTxnCommand_AppendNode_V1, IOET_WALTxnCommand_AppendNode_V2, } { - version := version objectio.RegisterIOEnrtyCodec( objectio.IOEntryHeader{ Type: IOET_WALTxnCommand_AppendNode, From cd122dbcb1085339c8e84bb0a559de6dda5cc6ae Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Wed, 29 Jul 2026 12:26:41 +0800 Subject: [PATCH 04/11] fix tae abort column CI regressions --- pkg/vm/engine/tae/blockio/read.go | 59 +++++---- .../engine/tae/index/indexwrapper/mutindex.go | 15 +++ pkg/vm/engine/tae/logtail/backup.go | 116 ++++++++++++------ pkg/vm/engine/tae/logtail/snapshot.go | 20 ++- pkg/vm/engine/test/change_handle_test.go | 23 +++- 5 files changed, 166 insertions(+), 67 deletions(-) diff --git a/pkg/vm/engine/tae/blockio/read.go b/pkg/vm/engine/tae/blockio/read.go index 39f99fe49e9c7..c2a838ff4b6ce 100644 --- a/pkg/vm/engine/tae/blockio/read.go +++ b/pkg/vm/engine/tae/blockio/read.go @@ -325,17 +325,6 @@ func CopyBlockData( return } -func windowCNBatch(bat *batch.Batch, start, end uint64) error { - var err error - for i, vec := range bat.Vecs { - bat.Vecs[i], err = vec.Window(int(start), int(end)) - if err != nil { - return err - } - } - return nil -} - func BlockDataReadBackup( ctx context.Context, info *objectio.BlockInfo, @@ -354,24 +343,40 @@ func BlockDataReadBackup( return } if !ts.IsEmpty() { - commitTs := types.TS{} - for v := 0; v < loaded.Vecs[0].Length(); v++ { - err = commitTs.Unmarshal(loaded.Vecs[len(loaded.Vecs)-1].GetRawBytesAt(v)) - if err != nil { - return + location := info.MetaLocation() + objectMeta, metaErr := objectio.FastLoadObjectMeta(ctx, &location, false, fs) + if metaErr != nil { + err = metaErr + return + } + blockMeta := objectMeta.MustDataMeta().GetBlockMeta(uint32(location.ID())) + layout := objectio.ResolveSpecialColumnLayout(blockMeta) + commitPos, ok := layout.Resolve(objectio.SEQNUM_COMMITTS) + if !ok { + err = moerr.NewInternalError(ctx, "backup object has no commit timestamp") + return + } + commitTSs := vector.MustFixedColWithTypeCheck[types.TS](loaded.Vecs[commitPos]) + var aborts []bool + if abortPos, ok := layout.Resolve(objectio.SEQNUM_ABORT); ok { + abortVec := loaded.Vecs[abortPos] + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) } - if commitTs.GT(&ts) { - err = windowCNBatch(loaded, 0, uint64(v)) - if err != nil { - return - } - logutil.Info("[BlockDataReadBackup]", - zap.String("commitTs", commitTs.ToString()), - zap.String("ts", ts.ToString()), - zap.String("location", info.MetaLocation().String()), - zap.Int("rows", v)) - break + } + visibleRows := make([]int64, 0, len(commitTSs)) + for row, commitTS := range commitTSs { + if commitTS.GT(&ts) || (aborts != nil && aborts[row]) { + continue } + visibleRows = append(visibleRows, int64(row)) + } + if len(visibleRows) != len(commitTSs) { + loaded.Shrink(visibleRows, false) + logutil.Info("[BlockDataReadBackup]", + zap.String("ts", ts.ToString()), + zap.String("location", info.MetaLocation().String()), + zap.Int("rows", len(visibleRows))) } } tombstones, err := ds.GetTombstones(ctx, &info.BlockID) diff --git a/pkg/vm/engine/tae/index/indexwrapper/mutindex.go b/pkg/vm/engine/tae/index/indexwrapper/mutindex.go index 176166e418834..e6b367a04c48b 100644 --- a/pkg/vm/engine/tae/index/indexwrapper/mutindex.go +++ b/pkg/vm/engine/tae/index/indexwrapper/mutindex.go @@ -171,6 +171,21 @@ func (idx *MutIndex) GetDuplicatedRows( if err == index.ErrNotFound { return nil } + // Preserve the conflict check against the newest non-aborted owner even + // when that row is newer than this transaction's snapshot. Aborted + // append nodes remain in the index as rollback holes and must be skipped. + if skipFn != nil { + for i := len(rows) - 1; i >= 0; i-- { + err = skipFn(rows[i]) + if err == index.ErrNotFound { + continue + } + if err != nil { + return err + } + break + } + } var maxRow uint32 exist := false for i := len(rows) - 1; i >= 0; i-- { diff --git a/pkg/vm/engine/tae/logtail/backup.go b/pkg/vm/engine/tae/logtail/backup.go index a5f534390e81d..59890d949957c 100644 --- a/pkg/vm/engine/tae/logtail/backup.go +++ b/pkg/vm/engine/tae/logtail/backup.go @@ -19,6 +19,7 @@ import ( "fmt" "math" + "github.com/matrixorigin/matrixone/pkg/common/moerr" "github.com/matrixorigin/matrixone/pkg/common/mpool" "github.com/matrixorigin/matrixone/pkg/container/batch" "github.com/matrixorigin/matrixone/pkg/container/types" @@ -59,6 +60,47 @@ type BackupDeltaLocDataSource struct { needShrink bool } +func loadSpecialColumnLayout( + ctx context.Context, + fs fileservice.FileService, + location objectio.Location, +) (objectio.SpecialColumnLayout, error) { + objectMeta, err := objectio.FastLoadObjectMeta(ctx, &location, false, fs) + if err != nil { + return objectio.SpecialColumnLayout{}, err + } + blockMeta := objectMeta.MustDataMeta().GetBlockMeta(uint32(location.ID())) + return objectio.ResolveSpecialColumnLayout(blockMeta), nil +} + +func visibleAppendableRows( + ctx context.Context, + bat *batch.Batch, + layout objectio.SpecialColumnLayout, + ts *types.TS, +) ([]int64, error) { + commitPos, ok := layout.Resolve(objectio.SEQNUM_COMMITTS) + if !ok || int(commitPos) >= len(bat.Vecs) { + return nil, moerr.NewInternalError(ctx, "appendable object has no commit timestamp") + } + commitTSs := vector.MustFixedColWithTypeCheck[types.TS](bat.Vecs[commitPos]) + var aborts []bool + if abortPos, ok := layout.Resolve(objectio.SEQNUM_ABORT); ok && int(abortPos) < len(bat.Vecs) { + abortVec := bat.Vecs[abortPos] + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + } + } + rows := make([]int64, 0, len(commitTSs)) + for row, commitTS := range commitTSs { + if (aborts != nil && aborts[row]) || (ts != nil && commitTS.GT(ts)) { + continue + } + rows = append(rows, int64(row)) + } + return rows, nil +} + func NewBackupDeltaLocDataSource( ctx context.Context, fs fileservice.FileService, @@ -226,25 +268,16 @@ func (d *BackupDeltaLocDataSource) GetTombstones( return false, err } if !tombstone.GetCNCreated() { - deleteRow := make([]int64, 0) - for v := 0; v < bat.Vecs[0].Length(); v++ { - var commitTs types.TS - err = commitTs.Unmarshal(bat.Vecs[len(bat.Vecs)-1].GetRawBytesAt(v)) - if err != nil { - return false, err - } - if commitTs.GT(&d.ts) { - - logutil.Debug("[GetSnapshot]", - zap.Int("row", v), - zap.String("commitTs", commitTs.ToString()), - zap.String("location", location.String())) - } else { - deleteRow = append(deleteRow, int64(v)) - } + layout, err := loadSpecialColumnLayout(ctx, d.fs, location) + if err != nil { + return false, err + } + visibleRows, err := visibleAppendableRows(ctx, bat, layout, &d.ts) + if err != nil { + return false, err } - if len(deleteRow) != bat.Vecs[0].Length() { - bat.Shrink(deleteRow, false) + if len(visibleRows) != bat.Vecs[0].Length() { + bat.Shrink(visibleRows, false) } } if id == 0 { @@ -341,27 +374,21 @@ func trimTombstoneData( var err error var sortKey uint16 // As long as there is an aBlk to be deleted, isCkpChange must be set to true. - commitTs := types.TS{} location.SetID(uint16(0)) bat, sortKey, err = ioutil.LoadOneBlock(ctx, fs, location, objectio.SchemaData) if err != nil { return err } - deleteRow := make([]int64, 0) - for v := 0; v < bat.Vecs[0].Length(); v++ { - err = commitTs.Unmarshal(bat.Vecs[len(bat.Vecs)-1].GetRawBytesAt(v)) - if err != nil { - return err - } - if commitTs.GT(&ts) { - logutil.Debugf("delete row %v, commitTs %v, location %v", - v, commitTs.ToString(), (*objectsData)[name].stats.ObjectLocation().String()) - } else { - deleteRow = append(deleteRow, int64(v)) - } + layout, err := loadSpecialColumnLayout(ctx, fs, location) + if err != nil { + return err } - if len(deleteRow) != bat.Vecs[0].Length() { - bat.Shrink(deleteRow, false) + visibleRows, err := visibleAppendableRows(ctx, bat, layout, &ts) + if err != nil { + return err + } + if len(visibleRows) != bat.Vecs[0].Length() { + bat.Shrink(visibleRows, false) } bat = formatData(bat) (*objectsData)[name].sortKey = sortKey @@ -750,6 +777,7 @@ func ReWriteCheckpointAndBlockFromKey( logutil.Info("[Data Empty] ReWrite Checkpoint", zap.String("object", oData.stats.ObjectName().String()), zap.Uint64("tid", oData.tid)) + bat.Clean(common.DebugAllocator) return true, nil } oData.sortKey = sortKey @@ -758,9 +786,25 @@ func ReWriteCheckpointAndBlockFromKey( if oData.sortKey != math.MaxUint16 { writer.SetPrimaryKey(oData.sortKey) } - result := batch.NewWithSize(len(oData.data[0].Vecs) - 2) - for i := range result.Vecs { - result.Vecs[i] = oData.data[0].Vecs[i] + layout, err := loadSpecialColumnLayout(ctx, fs, oData.stats.ObjectLocation()) + if err != nil { + return true, err + } + specialPositions := map[uint16]struct{}{} + for _, pos := range []uint16{layout.PhysicalAddr, layout.CommitTS, layout.Abort} { + if pos != objectio.InvalidSpecialColumnPosition { + specialPositions[pos] = struct{}{} + } + } + result := batch.NewWithSize(len(oData.data[0].Vecs) - len(specialPositions)) + resultPos := 0 + for pos, vec := range oData.data[0].Vecs { + if _, special := specialPositions[uint16(pos)]; special { + vec.Free(common.DebugAllocator) + continue + } + result.Vecs[resultPos] = vec + resultPos++ } result = formatData(result) oData.data[0] = result diff --git a/pkg/vm/engine/tae/logtail/snapshot.go b/pkg/vm/engine/tae/logtail/snapshot.go index 58a031f9966cf..08c7f28512221 100644 --- a/pkg/vm/engine/tae/logtail/snapshot.go +++ b/pkg/vm/engine/tae/logtail/snapshot.go @@ -730,9 +730,27 @@ func (sm *SnapshotMeta) updateTableInfo( return err } - commitTSs := vector.MustFixedColWithTypeCheck[types.TS](objectBat.Vecs[len(objectBat.Vecs)-1]) + layout, err := loadSpecialColumnLayout(ctx, fs, obj.stats.ObjectLocation()) + if err != nil { + return err + } + commitPos, ok := layout.Resolve(objectio.SEQNUM_COMMITTS) + if !ok { + return moerr.NewInternalError(ctx, "snapshot tombstone object has no commit timestamp") + } + commitTSs := vector.MustFixedColWithTypeCheck[types.TS](objectBat.Vecs[commitPos]) + var aborts []bool + if abortPos, ok := layout.Resolve(objectio.SEQNUM_ABORT); ok { + abortVec := objectBat.Vecs[abortPos] + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + } + } rowIDs := vector.MustFixedColWithTypeCheck[types.Rowid](objectBat.Vecs[0]) for i := 0; i < len(commitTSs); i++ { + if aborts != nil && aborts[i] { + continue + } pk, _, _, _ := types.DecodeTuple(objectBat.Vecs[1].GetRawBytesAt(i)) commitTs := commitTSs[i] if commitTs.LT(&startts) || commitTs.GT(&endts) { diff --git a/pkg/vm/engine/test/change_handle_test.go b/pkg/vm/engine/test/change_handle_test.go index 57f3b7ff8e811..71718fc4577f5 100644 --- a/pkg/vm/engine/test/change_handle_test.go +++ b/pkg/vm/engine/test/change_handle_test.go @@ -397,13 +397,30 @@ func checkInsertBatch(userBatch *containers.Batch, bat *batch.Batch, t *testing. return } length := bat.RowCount() - assert.Equal(t, len(bat.Vecs), len(userBatch.Vecs)+1) // user rows + committs + require.GreaterOrEqual(t, len(bat.Vecs), len(userBatch.Vecs)+1) for i, vec := range userBatch.Vecs { assert.Equal(t, bat.Vecs[i].GetType().Oid, vec.GetType().Oid) assert.Equal(t, bat.Vecs[i].Length(), length) } - assert.Equal(t, bat.Vecs[len(userBatch.Vecs)].GetType().Oid, types.T_TS) - assert.Equal(t, bat.Vecs[len(userBatch.Vecs)].Length(), length) + commitPos := -1 + for pos, attr := range bat.Attrs { + if attr == objectio.DefaultCommitTS_Attr { + commitPos = pos + break + } + } + if commitPos == -1 { + for pos := len(bat.Vecs) - 1; pos >= len(userBatch.Vecs); pos-- { + if bat.Vecs[pos].GetType().Oid == types.T_TS { + commitPos = pos + break + } + } + } + require.NotEqual(t, -1, commitPos) + require.Less(t, commitPos, len(bat.Vecs)) + assert.Equal(t, types.T_TS, bat.Vecs[commitPos].GetType().Oid) + assert.Equal(t, length, bat.Vecs[commitPos].Length()) } func changesHandleTestRowCount() int { From 63c9a8981d1a70f4a682235943812668b8cd9e17 Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Mon, 3 Aug 2026 16:09:31 +0800 Subject: [PATCH 05/11] fix tae abort visibility regressions --- pkg/objectio/constructors.go | 32 +++++++ pkg/objectio/ioutil/loadfuncs.go | 31 +++++-- pkg/objectio/ioutil/loadfuncs_test.go | 83 +++++++++++++++++++ .../engine/tae/index/indexwrapper/mutindex.go | 20 +++-- .../tae/index/indexwrapper/mutindex_test.go | 69 +++++++++++++++ pkg/vm/engine/tae/tables/functions.go | 40 +++++---- pkg/vm/engine/tae/tables/functions_test.go | 41 +++++++++ 7 files changed, 284 insertions(+), 32 deletions(-) create mode 100644 pkg/vm/engine/tae/index/indexwrapper/mutindex_test.go diff --git a/pkg/objectio/constructors.go b/pkg/objectio/constructors.go index 8573cd3e666f1..093d9a72bcd8d 100644 --- a/pkg/objectio/constructors.go +++ b/pkg/objectio/constructors.go @@ -594,6 +594,17 @@ func FilterCachedRowsByCommitTS( data fscache.Data, sels []int64, snapshot types.TS, +) ([]int64, error) { + return FilterCachedRowsByCommitTSAndAbort(data, nil, sels, snapshot) +} + +// FilterCachedRowsByCommitTSAndAbort removes rows newer than snapshot and rows +// marked aborted. A nil or const-null abort vector is the legacy object format. +func FilterCachedRowsByCommitTSAndAbort( + data fscache.Data, + abortData fscache.Data, + sels []int64, + snapshot types.TS, ) ([]int64, error) { var commits vector.Vector if err := bindCachedVectorForScope(&commits, data); err != nil { @@ -603,6 +614,19 @@ func FilterCachedRowsByCommitTS( if commits.GetType().Oid != types.T_TS || commits.IsConstNull() { return nil, moerr.NewInvalidInputNoCtx("object commit-ts column is unavailable") } + var aborts vector.Vector + hasAborts := abortData != nil + if hasAborts { + if err := bindCachedVectorForScope(&aborts, abortData); err != nil { + return nil, err + } + defer aborts.Free(nil) + if aborts.IsConstNull() { + hasAborts = false + } else if aborts.GetType().Oid != types.T_bool || aborts.Length() != commits.Length() { + return nil, moerr.NewInvalidInputNoCtx("object abort column is unavailable") + } + } filtered := sels[:0] for _, sel := range sels { @@ -616,6 +640,14 @@ func FilterCachedRowsByCommitTS( if commits.IsNull(uint64(sel)) { return nil, moerr.NewInvalidInputNoCtxf("object commit-ts row %d is null", sel) } + if hasAborts { + if aborts.IsNull(uint64(sel)) { + return nil, moerr.NewInvalidInputNoCtxf("object abort row %d is null", sel) + } + if vector.GetFixedAtNoTypeCheck[bool](&aborts, int(sel)) { + continue + } + } commit := vector.GetFixedAtNoTypeCheck[types.TS](&commits, int(sel)) if !commit.GT(&snapshot) { filtered = append(filtered, sel) diff --git a/pkg/objectio/ioutil/loadfuncs.go b/pkg/objectio/ioutil/loadfuncs.go index 9bcb0de82a5a6..475fcde557590 100644 --- a/pkg/objectio/ioutil/loadfuncs.go +++ b/pkg/objectio/ioutil/loadfuncs.go @@ -105,12 +105,12 @@ func readColumnsData( readColumns := columns readTypes := typs if extraTSColumn != nil { - readColumns = make([]uint16, len(columns), len(columns)+1) + readColumns = make([]uint16, len(columns), len(columns)+2) copy(readColumns, columns) - readColumns = append(readColumns, *extraTSColumn) - readTypes = make([]types.Type, len(typs), len(typs)+1) + readColumns = append(readColumns, *extraTSColumn, objectio.SEQNUM_ABORT) + readTypes = make([]types.Type, len(typs), len(typs)+2) copy(readTypes, typs) - readTypes = append(readTypes, objectio.TSType) + readTypes = append(readTypes, objectio.TSType, types.T_bool.ToType()) } name := location.Name().UnsafeString() @@ -231,6 +231,19 @@ func LoadColumnsDataInto( err = moerr.NewInvalidInputNoCtx("object commit-ts column is unavailable") return } + var aborts vector.Vector + if err = objectio.MustVectorToCached( + &aborts, + ioVectors.Entries[len(columns)+1].CachedData, + ); err != nil { + return + } + defer aborts.Free(nil) + hasAborts := !aborts.IsConstNull() + if hasAborts && (aborts.GetType().Oid != types.T_bool || aborts.Length() != commits.Length()) { + err = moerr.NewInvalidInputNoCtx("object abort column is unavailable") + return + } deleteMask = objectio.GetReusableBitmap() for i := 0; i < commits.Length(); i++ { @@ -238,8 +251,13 @@ func LoadColumnsDataInto( err = moerr.NewInvalidInputNoCtxf("object commit-ts row %d is null", i) return } + if hasAborts && aborts.IsNull(uint64(i)) { + err = moerr.NewInvalidInputNoCtxf("object abort row %d is null", i) + return + } commit := vector.GetFixedAtNoTypeCheck[types.TS](&commits, i) - if commit.GT(visibilityTS) { + if commit.GT(visibilityTS) || + (hasAborts && vector.GetFixedAtNoTypeCheck[bool](&aborts, i)) { deleteMask.Add(uint64(i)) } } @@ -314,8 +332,9 @@ func LoadColumnDataBySearch( return nil, false, err } if visibilityTS != nil { - sels, err = objectio.FilterCachedRowsByCommitTS( + sels, err = objectio.FilterCachedRowsByCommitTSAndAbort( ioVectors.Entries[1].CachedData, + ioVectors.Entries[2].CachedData, sels, *visibilityTS, ) diff --git a/pkg/objectio/ioutil/loadfuncs_test.go b/pkg/objectio/ioutil/loadfuncs_test.go index 10699c70f7f01..7df6be4c9128c 100644 --- a/pkg/objectio/ioutil/loadfuncs_test.go +++ b/pkg/objectio/ioutil/loadfuncs_test.go @@ -68,6 +68,89 @@ type releaseTrackingData struct { outstanding *atomic.Int32 } +func TestAppendableVisibilityFiltersAbortFromMaterializeAndSearch(t *testing.T) { + ctx := context.Background() + fs := testutil.NewSharedFS() + mp := mpool.MustNewZero() + defer mpool.DeleteMPool(mp) + + input := batch.NewWithSize(3) + input.Vecs[0] = vector.NewVec(types.T_varchar.ToType()) + input.Vecs[1] = vector.NewVec(types.T_TS.ToType()) + input.Vecs[2] = vector.NewVec(types.T_bool.ToType()) + defer input.Clean(mp) + for _, row := range []struct { + key string + commit types.TS + aborted bool + }{ + {key: "live", commit: types.BuildTS(5, 0)}, + {key: "aborted", commit: types.BuildTS(6, 0), aborted: true}, + {key: "future", commit: types.BuildTS(20, 0)}, + } { + require.NoError(t, vector.AppendBytes(input.Vecs[0], []byte(row.key), false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[1], row.commit, false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[2], row.aborted, false, mp)) + } + input.SetRowCount(3) + + writer := ConstructWriter( + 0, + []uint16{0, objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT}, + -1, + false, + false, + fs, + ) + writer.SetAppendable() + _, err := writer.WriteBatch(input) + require.NoError(t, err) + _, _, err = writer.Sync(ctx) + require.NoError(t, err) + stats := writer.GetObjectStats(objectio.WithAppendable()) + location := stats.ObjectLocation() + snapshot := types.BuildTS(10, 0) + + destination := vector.NewVec(types.T_varchar.ToType()) + defer destination.Free(mp) + deleteMask, _, err := LoadColumnsDataInto( + ctx, + []uint16{0}, + []types.Type{types.T_varchar.ToType()}, + fs, + location, + []*vector.Vector{destination}, + nil, + &snapshot, + mp, + fileservice.Policy(0), + ) + require.NoError(t, err) + defer deleteMask.Release() + require.True(t, deleteMask.Contains(1), "aborted row must be hidden from full scans") + require.True(t, deleteMask.Contains(2), "future row must be hidden from full scans") + require.False(t, deleteMask.Contains(0)) + + search := objectio.NewReadFilterSearch( + types.T_varchar, + [][]byte{[]byte("live"), []byte("aborted"), []byte("future")}, + ) + sels, _, err := LoadColumnDataBySearch( + ctx, + 0, + types.T_varchar.ToType(), + fs, + location, + search, + false, + &snapshot, + mp, + fileservice.Policy(0), + ) + require.NoError(t, err) + require.Equal(t, []int64{0}, sels, "cached search must return only live visible rows") +} + func (d *releaseTrackingData) Slice(length int) fscache.Data { d.Data = d.Data.Slice(length) return d diff --git a/pkg/vm/engine/tae/index/indexwrapper/mutindex.go b/pkg/vm/engine/tae/index/indexwrapper/mutindex.go index e6b367a04c48b..d14836c6c64ce 100644 --- a/pkg/vm/engine/tae/index/indexwrapper/mutindex.go +++ b/pkg/vm/engine/tae/index/indexwrapper/mutindex.go @@ -246,17 +246,19 @@ func (idx *MutIndex) Contains( if err == index.ErrNotFound { return nil } - if len(rows) != 1 { - panic("logic err: tombstones doesn't have duplicate rows") - } - err = skipFn(rows[0]) - if err == index.ErrNotFound { + for i := len(rows) - 1; i >= 0; i-- { + if skipFn != nil { + err = skipFn(rows[i]) + if err == index.ErrNotFound { + continue + } + if err != nil { + return err + } + } + containers.UpdateValue(keys, uint32(offset), nil, true, mp) return nil } - if err != nil { - return err - } - containers.UpdateValue(keys, uint32(offset), nil, true, mp) return nil } if err = containers.ForeachWindowBytes(keys, 0, keys.Length(), op, nil); err != nil { diff --git a/pkg/vm/engine/tae/index/indexwrapper/mutindex_test.go b/pkg/vm/engine/tae/index/indexwrapper/mutindex_test.go new file mode 100644 index 0000000000000..bf0b214b4504a --- /dev/null +++ b/pkg/vm/engine/tae/index/indexwrapper/mutindex_test.go @@ -0,0 +1,69 @@ +// Copyright 2026 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package indexwrapper + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/matrixorigin/matrixone/pkg/common/mpool" + "github.com/matrixorigin/matrixone/pkg/container/types" + "github.com/matrixorigin/matrixone/pkg/container/vector" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/index" +) + +func TestContainsSkipsDuplicateAbortedOffsets(t *testing.T) { + mp := mpool.MustNewZero() + defer mpool.DeleteMPool(mp) + idx := NewMutIndex(types.T_int32.ToType()) + + insert := func(offset int) { + vec := vector.NewVec(types.T_int32.ToType()) + require.NoError(t, vector.AppendFixed(vec, int32(7), false, mp)) + require.NoError(t, idx.BatchUpsert(vec, offset)) + vec.Free(mp) + } + insert(0) + insert(1) + + check := func(skipFn func(uint32) error) bool { + keys := vector.NewVec(types.T_int32.ToType()) + require.NoError(t, vector.AppendFixed(keys, int32(7), false, mp)) + err := idx.Contains( + context.Background(), + keys, + index.NewZM(types.T_int32, 0), + &types.Blockid{}, + skipFn, + mp, + ) + require.NoError(t, err) + deleted := keys.IsNull(0) + keys.Free(mp) + return deleted + } + + require.True(t, check(func(row uint32) error { + if row == 0 { + return index.ErrNotFound + } + return nil + }), "a live retry offset must still classify the key as deleted") + require.False(t, check(func(uint32) error { + return index.ErrNotFound + }), "all-aborted offsets must leave the key visible") +} diff --git a/pkg/vm/engine/tae/tables/functions.go b/pkg/vm/engine/tae/tables/functions.go index c3a18e3468ef3..e89464a3be1fa 100644 --- a/pkg/vm/engine/tae/tables/functions.go +++ b/pkg/vm/engine/tae/tables/functions.go @@ -467,25 +467,31 @@ func containsABlkFuncFactory[T types.FixedSizeT](comp func(T, T) int) func(args return err } } - rowIDs.Update(rowOffset, nil, true) - if isAborted(abortVec, row) { - return nil + first := row + for first > 0 && comp(vs[first-1], v1) == 0 { + first-- } - commitTS := tsVec.Get(row).(types.TS) - startTS := txn.GetStartTS() - if commitTS.GT(&startTS) { - ts, err := delsFn(v1, commitTS) - if err != nil { - return err + for row = first; row < len(vs) && comp(vs[row], v1) == 0; row++ { + if isAborted(abortVec, row) { + continue } - if ts.GT(&startTS) { - logutil.Info("Dedup-WW", - zap.String("txn", txn.Repr()), - zap.Int("row offset", row), - zap.String("commit ts", commitTS.ToString()), - zap.String("original commit ts", ts.ToString()), - ) - return txnif.ErrTxnWWConflict + rowIDs.Update(rowOffset, nil, true) + commitTS := tsVec.Get(row).(types.TS) + startTS := txn.GetStartTS() + if commitTS.GT(&startTS) { + ts, err := delsFn(v1, commitTS) + if err != nil { + return err + } + if ts.GT(&startTS) { + logutil.Info("Dedup-WW", + zap.String("txn", txn.Repr()), + zap.Int("row offset", row), + zap.String("commit ts", commitTS.ToString()), + zap.String("original commit ts", ts.ToString()), + ) + return txnif.ErrTxnWWConflict + } } } } diff --git a/pkg/vm/engine/tae/tables/functions_test.go b/pkg/vm/engine/tae/tables/functions_test.go index 54f7a88ae92cc..e04c6bc653334 100644 --- a/pkg/vm/engine/tae/tables/functions_test.go +++ b/pkg/vm/engine/tae/tables/functions_test.go @@ -109,6 +109,47 @@ func TestPersistedAppendableDedupSkipsAbortedRow(t *testing.T) { require.True(t, rowIDs.IsNull(0)) } +func TestPersistedTombstoneContainsSkipsAbortedMatches(t *testing.T) { + var blk types.Blockid + target := types.NewRowid(&blk, 7) + txn := txnbase.MockTxnReaderWithStartTS(types.BuildTS(10, 0)) + + check := func(aborts []bool) bool { + persisted := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) + defer persisted.Close() + commitTS := containers.MakeVector(types.T_TS.ToType(), common.DefaultAllocator) + abortVec := containers.MakeVector(types.T_bool.ToType(), common.DefaultAllocator) + for _, aborted := range aborts { + persisted.Append(target, false) + commitTS.Append(types.BuildTS(5, 0), false) + abortVec.Append(aborted, false) + } + keys := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) + defer keys.Close() + keys.Append(target, false) + + op := containers.MakeForeachVectorOp( + types.T_Rowid, + containsAlkFunctions, + persisted, + keys, + func(uint16) (containers.Vector, containers.Vector, error) { + return commitTS, abortVec, nil + }, + txn, + func(any, types.TS) (types.TS, error) { + t.Fatal("old committed tombstone must not invoke WW lookup") + return types.TS{}, nil + }, + ) + require.NoError(t, containers.ForeachVector(keys, op, nil)) + return keys.IsNull(0) + } + + require.True(t, check([]bool{true, false}), "a live physical match must delete the data row") + require.False(t, check([]bool{true, true}), "all-aborted physical matches must not delete the data row") +} + func TestMissingCommitTSFollowsDedupPolicy(t *testing.T) { for _, typ := range []types.Type{ types.T_int64.ToType(), From 269c20b1ba7225b548cc5cf35047f73572a9f0da Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Mon, 3 Aug 2026 17:20:40 +0800 Subject: [PATCH 06/11] fix mixed-version aobject abort layout --- pkg/defines/const.go | 3 +- pkg/sql/plan/function/ctl/cmd_rpc_version.go | 1 + .../plan/function/ctl/cmd_rpc_version_test.go | 17 +++++++- pkg/vm/engine/tae/db/dbutils/runtime.go | 22 ++++++++++- pkg/vm/engine/tae/tables/mnode.go | 39 +++++++++++++++---- pkg/vm/engine/tae/tables/object_test.go | 32 ++++++++++++++- 6 files changed, 102 insertions(+), 12 deletions(-) diff --git a/pkg/defines/const.go b/pkg/defines/const.go index 935071ecfe572..5d62ea45847cf 100644 --- a/pkg/defines/const.go +++ b/pkg/defines/const.go @@ -44,7 +44,8 @@ const ( MORPCVersion6 int64 = 6 // ordered aggregate pipeline configuration MORPCVersion7 int64 = 7 // structured CHECK constraint metadata and enforcement MORPCVersion8 int64 = 8 // versioned exact runtime-filter key contract - MORPCLatestVersion = MORPCVersion8 + MORPCVersion9 int64 = 9 // persisted appendable-object abort metadata + MORPCLatestVersion = MORPCVersion9 ) // DefaultLockWaitTimeoutSeconds is shared by the frontend default and by diff --git a/pkg/sql/plan/function/ctl/cmd_rpc_version.go b/pkg/sql/plan/function/ctl/cmd_rpc_version.go index 227161c154e80..8dcff5b268dac 100644 --- a/pkg/sql/plan/function/ctl/cmd_rpc_version.go +++ b/pkg/sql/plan/function/ctl/cmd_rpc_version.go @@ -163,6 +163,7 @@ func transferToTN(qt qclient.QueryClient, version int64) (Result, error) { if t.QueryAddress == "" { return true } + addr = t.QueryAddress ctx, cancel := context.WithTimeoutCause(context.Background(), time.Second*10, moerr.CauseTransferToTN) defer cancel() req := qt.NewRequest(querypb.CmdMethod_SetProtocolVersion) diff --git a/pkg/sql/plan/function/ctl/cmd_rpc_version_test.go b/pkg/sql/plan/function/ctl/cmd_rpc_version_test.go index 22a74e9b29865..1fd53a045b270 100644 --- a/pkg/sql/plan/function/ctl/cmd_rpc_version_test.go +++ b/pkg/sql/plan/function/ctl/cmd_rpc_version_test.go @@ -15,6 +15,7 @@ package ctl import ( + "context" "fmt" "testing" "time" @@ -29,6 +30,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/common/runtime" "github.com/matrixorigin/matrixone/pkg/defines" "github.com/matrixorigin/matrixone/pkg/pb/metadata" + "github.com/matrixorigin/matrixone/pkg/pb/query" "github.com/matrixorigin/matrixone/pkg/queryservice" qclient "github.com/matrixorigin/matrixone/pkg/queryservice/client" "github.com/matrixorigin/matrixone/pkg/util/trace" @@ -138,6 +140,18 @@ func requireVersionValue(t *testing.T, version int64) { require.EqualValues(t, version, v) } +type addressRecordingQueryClient struct { + testQClient + address string +} + +func (c *addressRecordingQueryClient) SendMessage( + _ context.Context, address string, _ *query.Request, +) (*query.Response, error) { + c.address = address + return nil, moerr.NewInternalErrorNoCtx("send error") +} + func Test_transferToTN(t *testing.T) { rt := runtime.DefaultRuntime() @@ -159,7 +173,8 @@ func Test_transferToTN(t *testing.T) { defer mc.Close() rt.SetGlobalVariables(runtime.ClusterService, mc) - qcli := &testQClient{} + qcli := &addressRecordingQueryClient{} _, err := transferToTN(qcli, 0) assert.Error(t, err) + assert.Equal(t, "wrong address", qcli.address) } diff --git a/pkg/vm/engine/tae/db/dbutils/runtime.go b/pkg/vm/engine/tae/db/dbutils/runtime.go index 1171ff7f67a80..e6cbbd6002802 100644 --- a/pkg/vm/engine/tae/db/dbutils/runtime.go +++ b/pkg/vm/engine/tae/db/dbutils/runtime.go @@ -20,7 +20,9 @@ import ( "sync" "time" + "github.com/matrixorigin/matrixone/pkg/common/runtime" "github.com/matrixorigin/matrixone/pkg/container/types" + "github.com/matrixorigin/matrixone/pkg/defines" "github.com/matrixorigin/matrixone/pkg/fileservice" "github.com/matrixorigin/matrixone/pkg/logutil" "github.com/matrixorigin/matrixone/pkg/util/metric/stats" @@ -194,12 +196,30 @@ func (r *Runtime) fillDefaults() { } func (r *Runtime) SID() string { - if r == nil { + if r == nil || r.Options == nil { return "" } return r.Options.SID } +// PersistedAObjectAbortSupported reports whether every reader in the current +// rollout understands appendable objects with the persisted abort column. +// Deployment keeps MOProtocolVersion at the oldest live service during a +// rolling upgrade, so writers must retain the legacy layout until version 9 is +// active. +func (r *Runtime) PersistedAObjectAbortSupported() bool { + serviceRuntime := runtime.ServiceRuntime(r.SID()) + if serviceRuntime == nil { + return false + } + value, ok := serviceRuntime.GetGlobalVariables(runtime.MOProtocolVersion) + if !ok { + return false + } + version, ok := value.(int64) + return ok && version >= defines.MORPCVersion9 +} + func (r *Runtime) PoolUsageReport() { var w bytes.Buffer w.WriteString(r.VectorPool.Transient.String()) diff --git a/pkg/vm/engine/tae/tables/mnode.go b/pkg/vm/engine/tae/tables/mnode.go index b7d4c720bc055..e712e823e41a2 100644 --- a/pkg/vm/engine/tae/tables/mnode.go +++ b/pkg/vm/engine/tae/tables/mnode.go @@ -162,24 +162,47 @@ func (node *memoryNode) getDataWindowOnWriteSchema( return nil } dest, ok := batches[node.writeSchema.Version] + persistAbort := node.object.rt.PersistedAObjectAbortSupported() if ok { - dest.Extend(node.data.Window(int(from), int(to-from))) - dest.GetVectorByName(objectio.TombstoneAttr_CommitTs_Attr).Extend(commitTSVec) - dest.GetVectorByName(objectio.TombstoneAttr_Abort_Attr).Extend(abort) - commitTSVec.Close() // TODO no copy + // One range collection may visit multiple objects while the rollout gate + // changes. Keep the schema chosen by the first batch internally stable. + _, persistAbort = dest.Nameidx[objectio.TombstoneAttr_Abort_Attr] + } + inner := node.data.CloneWindowWithPool( + int(from), int(to-from), node.object.rt.VectorPool.Transient) + inner.AddVector(objectio.TombstoneAttr_CommitTs_Attr, commitTSVec) + if persistAbort { + inner.AddVector(objectio.TombstoneAttr_Abort_Attr, abort) + } else { + // Old readers interpret every non-rowid/non-TS physical column as user + // data. Keep their commitTS-only layout and remove rollback holes before + // the batch can be flushed or published through logtail. + aborts := vector.MustFixedColWithTypeCheck[bool](abort.GetDownstreamVector()) + for row, aborted := range aborts { + if aborted { + inner.Delete(row) + } + } + inner.Compact() abort.Close() + } + if inner.Length() == 0 { + inner.Close() + return nil + } + if ok { + dest.Extend(inner) } else { - inner := node.data.CloneWindowWithPool(int(from), int(to-from), node.object.rt.VectorPool.Transient) batWithVer := &containers.BatchWithVersion{ Version: node.writeSchema.Version, NextSeqnum: uint16(node.writeSchema.Extra.NextColSeqnum), Seqnums: node.writeSchema.AllSeqnums(), Batch: inner, } - inner.AddVector(objectio.TombstoneAttr_CommitTs_Attr, commitTSVec) - inner.AddVector(objectio.TombstoneAttr_Abort_Attr, abort) batWithVer.Seqnums = append(batWithVer.Seqnums, objectio.SEQNUM_COMMITTS) - batWithVer.Seqnums = append(batWithVer.Seqnums, objectio.SEQNUM_ABORT) + if persistAbort { + batWithVer.Seqnums = append(batWithVer.Seqnums, objectio.SEQNUM_ABORT) + } batches[node.writeSchema.Version] = batWithVer } return diff --git a/pkg/vm/engine/tae/tables/object_test.go b/pkg/vm/engine/tae/tables/object_test.go index ce1b733016081..ff941c1584d78 100644 --- a/pkg/vm/engine/tae/tables/object_test.go +++ b/pkg/vm/engine/tae/tables/object_test.go @@ -19,8 +19,10 @@ import ( "testing" "github.com/matrixorigin/matrixone/pkg/common/mpool" + moruntime "github.com/matrixorigin/matrixone/pkg/common/runtime" "github.com/matrixorigin/matrixone/pkg/container/types" "github.com/matrixorigin/matrixone/pkg/container/vector" + "github.com/matrixorigin/matrixone/pkg/defines" "github.com/matrixorigin/matrixone/pkg/objectio" api "github.com/matrixorigin/matrixone/pkg/pb/api" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/catalog" @@ -198,8 +200,16 @@ func TestApplyAppendLockedPadsMissingColumnsForUpgradedSchema(t *testing.T) { }) } -func TestMemoryNodeScanMarksRollbackHoleDeleted(t *testing.T) { +func TestMemoryNodeRollbackHoleVisibilityAndWriteLayout(t *testing.T) { defer testutils.AfterTest(t)() + rt := moruntime.ServiceRuntime("") + originalVersion, hadVersion := rt.GetGlobalVariables(moruntime.MOProtocolVersion) + rt.SetGlobalVariables(moruntime.MOProtocolVersion, defines.MORPCVersion9) + defer func() { + if hadVersion { + rt.SetGlobalVariables(moruntime.MOProtocolVersion, originalVersion) + } + }() schema := catalog.MockSchema(1, 0) c := catalog.MockCatalog(nil) defer c.Close() @@ -260,6 +270,26 @@ func TestMemoryNodeScanMarksRollbackHoleDeleted(t *testing.T) { ) require.Equal(t, []bool{false, true, false}, aborts) + // During a rolling upgrade, the TN keeps the legacy layout and physically + // removes rollback holes so an old CN cannot expose them as user rows. + rt.SetGlobalVariables(moruntime.MOProtocolVersion, defines.MORPCVersion8) + legacyBatches := make(map[uint32]*containers.BatchWithVersion) + err = mnode.getDataWindowOnWriteSchema( + context.Background(), + legacyBatches, + types.TS{}, + types.BuildTS(3, 0), + common.DefaultAllocator, + ) + require.NoError(t, err) + legacyBatch := legacyBatches[schema.Version] + require.NotNil(t, legacyBatch) + defer legacyBatch.Close() + require.NotContains(t, legacyBatch.Seqnums, uint16(objectio.SEQNUM_ABORT)) + require.NotContains(t, legacyBatch.Nameidx, objectio.TombstoneAttr_Abort_Attr) + require.Contains(t, legacyBatch.Seqnums, uint16(objectio.SEQNUM_COMMITTS)) + require.Equal(t, 2, legacyBatch.Length()) + var output *containers.Batch err = mnode.Scan( context.Background(), From d891917815e73ddc6feb727c60cd6d90074c0333 Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Tue, 4 Aug 2026 10:04:25 +0800 Subject: [PATCH 07/11] fix abort row offset remapping --- pkg/defines/const.go | 10 +-- pkg/publication/filter_object.go | 4 +- pkg/publication/filter_object_batch.go | 46 ++++++++---- pkg/publication/filter_object_batch_test.go | 78 +++++++++++++++++++-- pkg/publication/task_executor_test.go | 4 +- pkg/vm/engine/tae/blockio/read.go | 25 ++++--- pkg/vm/engine/tae/blockio/read_test.go | 51 ++++++++++++++ 7 files changed, 180 insertions(+), 38 deletions(-) diff --git a/pkg/defines/const.go b/pkg/defines/const.go index 8e65880a6a87d..eb293a05e581b 100644 --- a/pkg/defines/const.go +++ b/pkg/defines/const.go @@ -38,11 +38,11 @@ const ( MORPCMinVersion int64 = math.MinInt64 MORPCVersion1 int64 = 1 MORPCVersion2 int64 = 2 - MORPCVersion3 int64 = 3 // start from 1.3.0 - MORPCVersion4 int64 = 4 // start from 2.0.1 - MORPCVersion5 int64 = 5 // assignment-aware CHAR/VARCHAR casts - MORPCVersion6 int64 = 6 // ordered aggregate pipeline configuration - MORPCVersion7 int64 = 7 // structured CHECK constraint metadata and enforcement + MORPCVersion3 int64 = 3 // start from 1.3.0 + MORPCVersion4 int64 = 4 // start from 2.0.1 + MORPCVersion5 int64 = 5 // assignment-aware CHAR/VARCHAR casts + MORPCVersion6 int64 = 6 // ordered aggregate pipeline configuration + MORPCVersion7 int64 = 7 // structured CHECK constraint metadata and enforcement MORPCVersion8 int64 = 8 // versioned exact runtime-filter key contract MORPCVersion9 int64 = 9 // AUTO_INCREMENT epoch-fenced commit MORPCVersion10 int64 = 10 // persisted appendable-object abort metadata diff --git a/pkg/publication/filter_object.go b/pkg/publication/filter_object.go index fa7b501014ff4..6806748157632 100644 --- a/pkg/publication/filter_object.go +++ b/pkg/publication/filter_object.go @@ -172,7 +172,7 @@ func filterAppendableObject( defer bat.Close() // Filter batch by snapshot TS - filteredBat, err := filterBatchBySnapshotTS(ctx, bat, snapshotTS, mp) + filteredBat, originalRowOffsets, err := filterBatchBySnapshotTS(ctx, bat, snapshotTS, mp) if err != nil { return nil, moerr.NewInternalErrorf(ctx, "failed to filter batch by snapshot TS: %v", err) } @@ -188,7 +188,7 @@ func filterAppendableObject( // Sort batch by primary key, remove commit TS column, write to file, and record ObjectStats // This is data object (not tombstone), so use SchemaData // Use new object name for appendable objects (keepOriginalName=false) - objStats, rowOffsetMap, err := createObjectFromBatch(ctx, filteredBat, stats, snapshotTS, isTombstone, localFS, mp, sortKeySeqnum, false) + objStats, rowOffsetMap, err := createObjectFromBatch(ctx, filteredBat, originalRowOffsets, stats, snapshotTS, isTombstone, localFS, mp, sortKeySeqnum, false) if err != nil { return nil, moerr.NewInternalErrorf(ctx, "failed to create object from batch: %v", err) } diff --git a/pkg/publication/filter_object_batch.go b/pkg/publication/filter_object_batch.go index 0a7a3570af6fc..8f3b8cfa3206d 100644 --- a/pkg/publication/filter_object_batch.go +++ b/pkg/publication/filter_object_batch.go @@ -394,27 +394,27 @@ func convertObjectToBatch( return bat, nil } -// filterBatchBySnapshotTS filters batch rows by snapshot TS -// For appendable objects, rows with commit TS >snapshot TS should be filtered out +// filterBatchBySnapshotTS removes appendable-object rows that are not visible at +// the snapshot, including rows whose persisted abort marker is set. func filterBatchBySnapshotTS( ctx context.Context, bat *containers.Batch, snapshotTS types.TS, mp *mpool.MPool, -) (*containers.Batch, error) { +) (*containers.Batch, []uint32, error) { if bat == nil { - return nil, nil + return nil, nil, nil } // Find the commit TS column commitTSVec := bat.GetVectorByName(objectio.TombstoneAttr_CommitTs_Attr) if commitTSVec == nil { - return nil, moerr.NewInternalErrorf(ctx, "commit TS column not found in batch") + return nil, nil, moerr.NewInternalErrorf(ctx, "commit TS column not found in batch") } // Verify the column type is TS if commitTSVec.GetType().Oid != types.T_TS { - return nil, moerr.NewInternalErrorf(ctx, "commit TS column type mismatch: expected TS, got %s", commitTSVec.GetType().String()) + return nil, nil, moerr.NewInternalErrorf(ctx, "commit TS column type mismatch: expected TS, got %s", commitTSVec.GetType().String()) } // Get commit TS values @@ -423,11 +423,11 @@ func filterBatchBySnapshotTS( if abortPos, ok := bat.Nameidx[objectio.TombstoneAttr_Abort_Attr]; ok { abortVec := bat.Vecs[abortPos] if abortVec.GetType().Oid != types.T_bool { - return nil, moerr.NewInternalErrorf(ctx, "abort column has invalid type") + return nil, nil, moerr.NewInternalErrorf(ctx, "abort column has invalid type") } aborts = vector.MustFixedColWithTypeCheck[bool](abortVec.GetDownstreamVector()) if len(aborts) != len(commitTSs) { - return nil, moerr.NewInternalErrorf( + return nil, nil, moerr.NewInternalErrorf( ctx, "abort column length %d does not match commit TS length %d", len(aborts), @@ -436,17 +436,21 @@ func filterBatchBySnapshotTS( } } - // Build bitmap of rows to delete (commit TS < snapshot TS) + // Build a bitmap of rows hidden at the snapshot and retain the original + // physical offset of every surviving row. deletes := roaring.New() + originalRowOffsets := make([]uint32, 0, len(commitTSs)) for i, ts := range commitTSs { if ts.GT(&snapshotTS) || (aborts != nil && aborts[i]) { deletes.Add(uint32(i)) + continue } + originalRowOffsets = append(originalRowOffsets, uint32(i)) } // If no rows to delete, return original batch if deletes.IsEmpty() { - return bat, nil + return bat, originalRowOffsets, nil } // Compact all vectors to remove deleted rows @@ -454,7 +458,7 @@ func filterBatchBySnapshotTS( vec.Compact(deletes) } - return bat, nil + return bat, originalRowOffsets, nil } // createObjectFromBatch sorts batch by primary key, removes commit TS column, @@ -466,6 +470,7 @@ func filterBatchBySnapshotTS( func createObjectFromBatch( ctx context.Context, bat *containers.Batch, + originalRowOffsets []uint32, originalStats *objectio.ObjectStats, snapshotTS types.TS, isTombstone bool, @@ -477,6 +482,18 @@ func createObjectFromBatch( if bat == nil || bat.Length() == 0 { return objectio.ObjectStats{}, nil, nil } + if originalRowOffsets == nil { + originalRowOffsets = make([]uint32, bat.Length()) + for row := range originalRowOffsets { + originalRowOffsets[row] = uint32(row) + } + } else if len(originalRowOffsets) != bat.Length() { + return objectio.ObjectStats{}, nil, moerr.NewInternalErrorf( + ctx, + "original row offset count %d does not match batch length %d", + len(originalRowOffsets), bat.Length(), + ) + } // Step 1: Convert to CN batch for sorting cnBat := containers.ToCNBatch(bat) @@ -495,10 +512,11 @@ func createObjectFromBatch( sort.Sort(false, false, true, sortedIdx, cnBat.Vecs[pkIdx]) // Build rowOffsetMap: maps original rowoffset to new rowoffset after sorting - // sortedIdx[newIdx] = oldIdx, so we need: rowOffsetMap[oldIdx] = newIdx + // sortedIdx[newIdx] = compactedIdx. Translate compactedIdx through the + // provenance returned by snapshot filtering before recording the mapping. rowOffsetMap := make(map[uint32]uint32, len(sortedIdx)) - for newIdx, oldIdx := range sortedIdx { - rowOffsetMap[uint32(oldIdx)] = uint32(newIdx) + for newIdx, compactedIdx := range sortedIdx { + rowOffsetMap[originalRowOffsets[compactedIdx]] = uint32(newIdx) } for i := 0; i < len(cnBat.Vecs); i++ { diff --git a/pkg/publication/filter_object_batch_test.go b/pkg/publication/filter_object_batch_test.go index f5f9101a0bd00..81ec52f1fe26d 100644 --- a/pkg/publication/filter_object_batch_test.go +++ b/pkg/publication/filter_object_batch_test.go @@ -23,6 +23,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/container/types" "github.com/matrixorigin/matrixone/pkg/container/vector" "github.com/matrixorigin/matrixone/pkg/objectio" + "github.com/matrixorigin/matrixone/pkg/testutil" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -133,9 +134,10 @@ func TestRewriteTombstoneRowidsBatch_WithMapping(t *testing.T) { // ============================================================ func TestFilterBatchBySnapshotTS_NilBatch(t *testing.T) { - result, err := filterBatchBySnapshotTS(context.Background(), nil, types.TS{}, nil) + result, offsets, err := filterBatchBySnapshotTS(context.Background(), nil, types.TS{}, nil) assert.NoError(t, err) assert.Nil(t, result) + assert.Nil(t, offsets) } func TestFilterBatchBySnapshotTS_NoDeletes(t *testing.T) { @@ -153,9 +155,10 @@ func TestFilterBatchBySnapshotTS_NoDeletes(t *testing.T) { bat.AddVector(objectio.TombstoneAttr_CommitTs_Attr, tsVec) snapshotTS := types.BuildTS(300, 0) - result, err := filterBatchBySnapshotTS(context.Background(), bat, snapshotTS, mp) + result, offsets, err := filterBatchBySnapshotTS(context.Background(), bat, snapshotTS, mp) assert.NoError(t, err) assert.Equal(t, 2, result.Length()) + assert.Equal(t, []uint32{0, 1}, offsets) bat.Close() } @@ -180,9 +183,10 @@ func TestFilterBatchBySnapshotTS_WithDeletes(t *testing.T) { bat.AddVector(objectio.TombstoneAttr_Abort_Attr, abortVec) snapshotTS := types.BuildTS(300, 0) - result, err := filterBatchBySnapshotTS(context.Background(), bat, snapshotTS, mp) + result, offsets, err := filterBatchBySnapshotTS(context.Background(), bat, snapshotTS, mp) assert.NoError(t, err) assert.Equal(t, 1, result.Length()) + assert.Equal(t, []uint32{0}, offsets) bat.Close() } @@ -200,12 +204,78 @@ func TestFilterBatchBySnapshotTS_AllDeleted(t *testing.T) { bat.AddVector(objectio.TombstoneAttr_CommitTs_Attr, tsVec) snapshotTS := types.BuildTS(100, 0) - result, err := filterBatchBySnapshotTS(context.Background(), bat, snapshotTS, mp) + result, offsets, err := filterBatchBySnapshotTS(context.Background(), bat, snapshotTS, mp) assert.NoError(t, err) assert.Equal(t, 0, result.Length()) + assert.Empty(t, offsets) bat.Close() } +func TestPublicationOffsetMapPreservesInteriorAbortProvenance(t *testing.T) { + mp, err := mpool.NewMPool("test", 0, mpool.NoFixed) + require.NoError(t, err) + defer mp.Free(nil) + + bat := containers.NewBatch() + pkVec := containers.MakeVector(types.T_int32.ToType(), mp) + commitTSVec := containers.MakeVector(types.T_TS.ToType(), mp) + abortVec := containers.MakeVector(types.T_bool.ToType(), mp) + for row, key := range []int32{20, 99, 10} { + pkVec.Append(key, false) + commitTSVec.Append(types.BuildTS(1, 0), false) + abortVec.Append(row == 1, false) + } + bat.AddVector("pk", pkVec) + bat.AddVector(objectio.TombstoneAttr_CommitTs_Attr, commitTSVec) + bat.AddVector(objectio.TombstoneAttr_Abort_Attr, abortVec) + + filtered, originalOffsets, err := filterBatchBySnapshotTS( + context.Background(), bat, types.BuildTS(2, 0), mp, + ) + require.NoError(t, err) + require.Equal(t, []uint32{0, 2}, originalOffsets) + + var originalStats objectio.ObjectStats + downstreamStats, rowOffsetMap, err := createObjectFromBatch( + context.Background(), + filtered, + originalOffsets, + &originalStats, + types.BuildTS(2, 0), + false, + testutil.NewSharedFS(), + mp, + 0, + false, + ) + require.NoError(t, err) + require.Equal(t, map[uint32]uint32{0: 1, 2: 0}, rowOffsetMap) + + upstreamObjID := types.NewObjectid() + deleteRowID := types.NewRowIDWithObjectIDBlkNumAndRowID(upstreamObjID, 0, 2) + rowIDVec := vector.NewVec(types.T_Rowid.ToType()) + require.NoError(t, vector.AppendFixed(rowIDVec, deleteRowID, false, mp)) + tombstoneBatch := &batch.Batch{Vecs: []*vector.Vector{rowIDVec}} + tombstoneBatch.SetRowCount(1) + defer tombstoneBatch.Clean(mp) + + aobjectMap := NewAObjectMap() + aobjectMap.Set(upstreamObjID.String(), &AObjectMapping{ + DownstreamStats: downstreamStats, + RowOffsetMap: rowOffsetMap, + }) + require.NoError(t, rewriteTombstoneRowidsBatch( + context.Background(), tombstoneBatch, aobjectMap, mp, + )) + rewritten := vector.GetFixedAtNoTypeCheck[types.Rowid](rowIDVec, 0) + require.Equal(t, uint32(0), rewritten.GetRowOffset()) + require.Equal( + t, + downstreamStats.ObjectName().ObjectId().String(), + rewritten.BorrowObjectID().String(), + ) +} + // ============================================================ // Tests for rewriteTombstoneRowids (containers.Batch version) // ============================================================ diff --git a/pkg/publication/task_executor_test.go b/pkg/publication/task_executor_test.go index bfc31e771636f..b06813fc9b226 100644 --- a/pkg/publication/task_executor_test.go +++ b/pkg/publication/task_executor_test.go @@ -268,13 +268,13 @@ func TestCompareTableDefs_DropColumn(t *testing.T) { // ==== filter_object_batch.go ==== func TestFilterBatchBySnapshotTS_NilBatchCov(t *testing.T) { - _, err := filterBatchBySnapshotTS(context.Background(), nil, types.TS{}, nil) + _, _, err := filterBatchBySnapshotTS(context.Background(), nil, types.TS{}, nil) assert.NoError(t, err) } func TestCreateObjectFromBatch_NilBatchCov(t *testing.T) { var stats objectio.ObjectStats - result, _, err := createObjectFromBatch(context.Background(), nil, &stats, types.TS{}, false, nil, nil, 0, false) + result, _, err := createObjectFromBatch(context.Background(), nil, nil, &stats, types.TS{}, false, nil, nil, 0, false) assert.NoError(t, err) assert.True(t, result.IsZero()) } diff --git a/pkg/vm/engine/tae/blockio/read.go b/pkg/vm/engine/tae/blockio/read.go index 630448cb662a5..1889692a21a8e 100644 --- a/pkg/vm/engine/tae/blockio/read.go +++ b/pkg/vm/engine/tae/blockio/read.go @@ -367,6 +367,11 @@ func BlockDataReadBackup( if err != nil { return } + tombstones, err := ds.GetTombstones(ctx, &info.BlockID) + if err != nil { + return + } + defer tombstones.Release() if !ts.IsEmpty() { location := info.MetaLocation() objectMeta, metaErr := objectio.FastLoadObjectMeta(ctx, &location, false, fs) @@ -391,7 +396,9 @@ func BlockDataReadBackup( } visibleRows := make([]int64, 0, len(commitTSs)) for row, commitTS := range commitTSs { - if commitTS.GT(&ts) || (aborts != nil && aborts[row]) { + if commitTS.GT(&ts) || + (aborts != nil && aborts[row]) || + tombstones.Contains(uint64(row)) { continue } visibleRows = append(visibleRows, int64(row)) @@ -403,16 +410,12 @@ func BlockDataReadBackup( zap.String("location", info.MetaLocation().String()), zap.Int("rows", len(visibleRows))) } - } - tombstones, err := ds.GetTombstones(ctx, &info.BlockID) - if err != nil { - return - } - defer tombstones.Release() - rows := tombstones.ToI64Array(nil) - if len(rows) > 0 { - logutil.Info("[BlockDataReadBackup Shrink]", zap.String("location", info.MetaLocation().String()), zap.Int("rows", len(rows))) - loaded.Shrink(rows, true) + } else { + rows := tombstones.ToI64Array(nil) + if len(rows) > 0 { + logutil.Info("[BlockDataReadBackup Shrink]", zap.String("location", info.MetaLocation().String()), zap.Int("rows", len(rows))) + loaded.Shrink(rows, true) + } } return } diff --git a/pkg/vm/engine/tae/blockio/read_test.go b/pkg/vm/engine/tae/blockio/read_test.go index e41378daf607f..e198a7d8a8c48 100644 --- a/pkg/vm/engine/tae/blockio/read_test.go +++ b/pkg/vm/engine/tae/blockio/read_test.go @@ -29,6 +29,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/testutil" "github.com/matrixorigin/matrixone/pkg/vectorindex/metric" "github.com/matrixorigin/matrixone/pkg/vm/engine" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/common" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" "github.com/stretchr/testify/require" ) @@ -423,6 +424,56 @@ func TestBlockDataReadInnerAppendableVisibility(t *testing.T) { require.Zero(t, queryMP.CurrNB()) } +func TestBlockDataReadBackupCombinesAbortAndTombstoneMasks(t *testing.T) { + ctx := context.Background() + fs := testutil.NewSharedFS() + mp := mpool.MustNewZero() + defer mpool.DeleteMPool(mp) + + input := batch.NewWithSize(3) + input.Vecs[0] = vector.NewVec(types.T_int32.ToType()) + input.Vecs[1] = vector.NewVec(objectio.TSType) + input.Vecs[2] = vector.NewVec(types.T_bool.ToType()) + for row := range 4 { + require.NoError(t, vector.AppendFixed(input.Vecs[0], int32(row), false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[1], types.BuildTS(1, 0), false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[2], row == 1, false, mp)) + } + input.SetRowCount(4) + writer := ioutil.ConstructWriter( + 0, + []uint16{0, objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT}, + -1, + false, + false, + fs, + ) + writer.SetAppendable() + _, err := writer.WriteBatch(input) + require.NoError(t, err) + _, _, err = writer.Sync(ctx) + require.NoError(t, err) + stats := writer.GetObjectStats(objectio.WithAppendable()) + info := stats.ConstructBlockInfo(0) + input.Clean(mp) + + loaded, _, err := BlockDataReadBackup( + ctx, + &info, + &blockReadTestDataSource{deleted: []uint64{2}}, + nil, + types.BuildTS(2, 0), + fs, + ) + require.NoError(t, err) + defer loaded.Clean(common.DebugAllocator) + require.Equal( + t, + []int32{0, 3}, + vector.MustFixedColWithTypeCheck[int32](loaded.Vecs[0]), + ) +} + func TestFillOutputBatchBySelectedRows(t *testing.T) { mp := mpool.MustNewZero() defer mpool.DeleteMPool(mp) From 30f350d169e8173cd9ced765214627d0b4c2991d Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Tue, 4 Aug 2026 14:37:10 +0800 Subject: [PATCH 08/11] fix(disttae): keep rowid leading in persisted changes --- .../disttae/logtailreplay/change_handle.go | 32 +++++++++--- .../logtailreplay/change_handle_test.go | 51 +++++++++++++++++++ 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/pkg/vm/engine/disttae/logtailreplay/change_handle.go b/pkg/vm/engine/disttae/logtailreplay/change_handle.go index 47d893bacee55..398df066b82b3 100755 --- a/pkg/vm/engine/disttae/logtailreplay/change_handle.go +++ b/pkg/vm/engine/disttae/logtailreplay/change_handle.go @@ -2950,6 +2950,10 @@ func updatePersistedDataBatch( rowIDPos := uint16(objectio.InvalidSpecialColumnPosition) if layout.PhysicalAddr != objectio.InvalidSpecialColumnPosition { rowIDPos = layout.PhysicalAddr + if int(rowIDPos) >= len(bat.Vecs) || bat.Vecs[rowIDPos] == nil || + bat.Vecs[rowIDPos].GetType().Oid != types.T_Rowid { + return moerr.NewInternalErrorNoCtx("persisted appendable object has an invalid rowid column") + } } commitTSVec := bat.Vecs[commitPos] @@ -2985,7 +2989,21 @@ func updatePersistedDataBatch( filteredVecs := make([]*vector.Vector, 0, len(bat.Vecs)-1) filteredAttrs := make([]string, 0, len(bat.Attrs)) commitTSAttr := objectio.DefaultCommitTS_Attr + var rowIDVec *vector.Vector rowIDKept := false + if retainRowID && rowIDPos == objectio.InvalidSpecialColumnPosition { + if blk == nil { + return moerr.NewInternalErrorNoCtx("persisted appendable object cannot synthesize rowid without block id") + } + rowIDVec = vector.NewVec(types.T_Rowid.ToType()) + for i := range commits { + if err := vector.AppendFixed(rowIDVec, types.NewRowid(blk, uint32(i)), false, mp); err != nil { + rowIDVec.Free(mp) + return err + } + } + rowIDKept = true + } for i, vec := range bat.Vecs { pos := uint16(i) switch { @@ -2997,11 +3015,8 @@ func updatePersistedDataBatch( vec.Free(mp) case pos == rowIDPos: if retainRowID { - filteredVecs = append(filteredVecs, vec) + rowIDVec = vec rowIDKept = true - if rebuildAttrs { - filteredAttrs = append(filteredAttrs, bat.Attrs[i]) - } } else { vec.Free(mp) } @@ -3012,6 +3027,12 @@ func updatePersistedDataBatch( } } } + if rowIDKept { + filteredVecs = append([]*vector.Vector{rowIDVec}, filteredVecs...) + if rebuildAttrs { + filteredAttrs = append([]string{catalog.Row_ID}, filteredAttrs...) + } + } filteredVecs = append(filteredVecs, commitTSVec) if rebuildAttrs { filteredAttrs = append(filteredAttrs, commitTSAttr) @@ -3026,9 +3047,6 @@ func updatePersistedDataBatch( if len(bat.Vecs) > 0 { bat.SetRowCount(bat.Vecs[0].Length()) } - if retainRowID && !rowIDKept { - return prependRowIDVectorIfNeeded(bat, blk, mp) - } return nil } func updateCNTombstoneBatch(bat *batch.Batch, committs types.TS, blk *types.Blockid, retainRowID bool, mp *mpool.MPool) error { diff --git a/pkg/vm/engine/disttae/logtailreplay/change_handle_test.go b/pkg/vm/engine/disttae/logtailreplay/change_handle_test.go index 2168fa9b73ff1..52a19062b42d3 100644 --- a/pkg/vm/engine/disttae/logtailreplay/change_handle_test.go +++ b/pkg/vm/engine/disttae/logtailreplay/change_handle_test.go @@ -347,6 +347,57 @@ func TestUpdateDataBatch_RetainsSynthesizedRowID(t *testing.T) { bat.Clean(mp) } +func TestUpdatePersistedDataBatch_RetainsLeadingRowID(t *testing.T) { + mp := mpool.MustNewZero() + defer mpool.DeleteMPool(mp) + + bat := batch.NewWithSize(5) + bat.SetAttributes([]string{"a", "b", catalog.Row_ID, objectio.DefaultCommitTS_Attr, objectio.DefaultAbort_Attr}) + bat.Vecs[0] = vector.NewVec(types.T_int32.ToType()) + bat.Vecs[1] = vector.NewVec(types.T_int32.ToType()) + bat.Vecs[2] = vector.NewVec(types.T_Rowid.ToType()) + bat.Vecs[3] = vector.NewVec(types.T_TS.ToType()) + bat.Vecs[4] = vector.NewVec(types.T_bool.ToType()) + blk := objectio.NewBlockid(objectio.NewSegmentid(), 0, 0) + for i, aborted := range []bool{false, true, false} { + require.NoError(t, vector.AppendFixed(bat.Vecs[0], int32(i+1), false, mp)) + require.NoError(t, vector.AppendFixed(bat.Vecs[1], int32((i+1)*10), false, mp)) + require.NoError(t, vector.AppendFixed( + bat.Vecs[2], types.NewRowid(blk, uint32(i)), false, mp)) + require.NoError(t, vector.AppendFixed( + bat.Vecs[3], types.BuildTS(int64(100+i), 0), false, mp)) + require.NoError(t, vector.AppendFixed(bat.Vecs[4], aborted, false, mp)) + } + bat.SetRowCount(3) + + layout := objectio.SpecialColumnLayout{ + PhysicalAddr: 2, + CommitTS: 3, + Abort: 4, + } + require.NoError(t, updatePersistedDataBatch( + bat, types.BuildTS(50, 0), types.BuildTS(150, 0), blk, layout, true, mp)) + + require.Equal(t, []string{ + catalog.Row_ID, "a", "b", objectio.DefaultCommitTS_Attr, + }, bat.Attrs) + require.Equal(t, []types.T{ + types.T_Rowid, types.T_int32, types.T_int32, types.T_TS, + }, []types.T{ + bat.Vecs[0].GetType().Oid, + bat.Vecs[1].GetType().Oid, + bat.Vecs[2].GetType().Oid, + bat.Vecs[3].GetType().Oid, + }) + require.Equal(t, 2, bat.RowCount()) + rowIDs := vector.MustFixedColNoTypeCheck[types.Rowid](bat.Vecs[0]) + require.Equal(t, uint32(0), rowIDs[0].GetRowOffset()) + require.Equal(t, uint32(2), rowIDs[1].GetRowOffset()) + require.Equal(t, []int32{1, 3}, vector.MustFixedColNoTypeCheck[int32](bat.Vecs[1])) + + bat.Clean(mp) +} + func TestAObjectHandleShouldReadBlock_UsesCachedPlan(t *testing.T) { obj := makeTestObjectEntry(t, 2, false, false, types.BuildTS(10, 0)) handle := &AObjectHandle{ From 10de16b384379d1c0d03067c6b1268ef26f4b225 Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Tue, 4 Aug 2026 16:42:05 +0800 Subject: [PATCH 09/11] fix(tae): preserve v9 aobject row coordinates --- pkg/vm/engine/tae/db/test/db_test.go | 90 +++++++++++++++++++++++++ pkg/vm/engine/tae/tables/mnode.go | 8 +-- pkg/vm/engine/tae/tables/object_test.go | 13 +++- 3 files changed, 104 insertions(+), 7 deletions(-) diff --git a/pkg/vm/engine/tae/db/test/db_test.go b/pkg/vm/engine/tae/db/test/db_test.go index 61a6fb4bb7cd7..e4831cc166d72 100644 --- a/pkg/vm/engine/tae/db/test/db_test.go +++ b/pkg/vm/engine/tae/db/test/db_test.go @@ -39,6 +39,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/container/nulls" "github.com/matrixorigin/matrixone/pkg/container/types" "github.com/matrixorigin/matrixone/pkg/container/vector" + "github.com/matrixorigin/matrixone/pkg/defines" "github.com/matrixorigin/matrixone/pkg/fileservice" "github.com/matrixorigin/matrixone/pkg/logutil" "github.com/matrixorigin/matrixone/pkg/objectio" @@ -4052,6 +4053,95 @@ func TestFlushTransferTombstonesRollback(t *testing.T) { tae.CheckRowsByScan(0, true) } +func TestV9FlushPreservesRowIDAcrossAbortHole(t *testing.T) { + defer testutils.AfterTest(t)() + ctx := context.Background() + serviceRuntime := runtime.ServiceRuntime("") + originalVersion, hadVersion := serviceRuntime.GetGlobalVariables(runtime.MOProtocolVersion) + serviceRuntime.SetGlobalVariables(runtime.MOProtocolVersion, defines.MORPCVersion9) + defer func() { + if hadVersion { + serviceRuntime.SetGlobalVariables(runtime.MOProtocolVersion, originalVersion) + } + }() + + opts := config.WithLongScanAndCKPOpts(nil) + tae := testutil.NewTestEngine(ctx, ModuleName, t, opts) + defer tae.Close() + schema := catalog.MockSchemaAll(3, 2) + schema.Extra.BlockMaxRows = 10 + schema.Extra.ObjectMaxBlocks = 2 + tae.BindSchema(schema) + rows := catalog.MockBatch(schema, 4) + defer rows.Close() + first := rows.CloneWindow(0, 1) + defer first.Close() + aborted := rows.CloneWindow(1, 1) + defer aborted.Close() + tail := rows.CloneWindow(2, 2) + defer tail.Close() + + tae.CreateRelAndAppend(first, true) + abortTxn, abortRel := tae.GetRelation() + require.NoError(t, abortRel.Append(ctx, aborted)) + // Drive the append through the same prepare/apply boundary used before WAL + // publication, then roll it back to leave a physical MVCC-owned hole. + require.NoError(t, abortTxn.PrePrepare(ctx)) + require.NoError(t, abortTxn.PreApplyCommit()) + require.NoError(t, abortTxn.Rollback(ctx)) + tae.DoAppend(tail) + + deleteTxn, deleteRel := tae.GetRelation() + sortKeyIdx := schema.GetSingleSortKeyIdx() + key := tail.Vecs[sortKeyIdx].Get(0) + id, offset, err := deleteRel.GetByFilter(ctx, handle.NewEQFilter(key)) + require.NoError(t, err) + require.Equal(t, uint32(2), offset, "rollback hole must retain its physical slot") + oldBlockID := id.BlockID + require.NoError(t, deleteRel.RangeDelete(id, offset, offset, handle.DT_Normal)) + require.NoError(t, deleteTxn.Commit(ctx)) + deleteTS := deleteTxn.GetCommitTS() + snapshotTxn, err := tae.StartTxnWithStartTSAndSnapshotTS(nil, deleteTS) + require.NoError(t, err) + snapshotTxn.BindAccessInfo(0, 0, 0) + snapshotRel := tae.GetRelationWithTxn(snapshotTxn) + + flushTxn, flushRel := tae.GetRelation() + dataMetas := testutil.GetAllAppendableMetas(flushRel, false) + tombstoneMetas := testutil.GetAllAppendableMetas(flushRel, true) + require.NotEmpty(t, dataMetas) + task, err := jobs.NewFlushTableTailTask( + nil, flushTxn, dataMetas, tombstoneMetas, tae.Runtime, + ) + require.NoError(t, err) + require.NoError(t, task.OnExec(ctx)) + require.NoError(t, flushTxn.Commit(ctx)) + + // The historical snapshot is held open while the aobject location is + // replaced, so it cannot use tombstones transferred by the later flush. + // Its tombstone still targets original offset 2; it must hide that row while + // preserving the later live row at offset 3. + var view *containers.Batch + err = tables.HybridScanByBlock( + ctx, + snapshotRel.GetMeta().(*catalog.TableEntry), + snapshotTxn, + &view, + schema, + []int{sortKeyIdx}, + &oldBlockID, + common.DefaultAllocator, + ) + require.NoError(t, err) + require.NotNil(t, view) + defer view.Close() + view.Compact() + require.Equal(t, 2, view.Length()) + require.Equal(t, rows.Vecs[sortKeyIdx].Get(0), view.Vecs[0].Get(0)) + require.Equal(t, rows.Vecs[sortKeyIdx].Get(3), view.Vecs[0].Get(1)) + require.NoError(t, snapshotTxn.Commit(ctx)) +} + func TestIncrementalDedupIgnoresOldRowsInTNRewrite(t *testing.T) { defer testutils.AfterTest(t)() ctx := context.Background() diff --git a/pkg/vm/engine/tae/tables/mnode.go b/pkg/vm/engine/tae/tables/mnode.go index e712e823e41a2..83788be320cae 100644 --- a/pkg/vm/engine/tae/tables/mnode.go +++ b/pkg/vm/engine/tae/tables/mnode.go @@ -175,15 +175,15 @@ func (node *memoryNode) getDataWindowOnWriteSchema( inner.AddVector(objectio.TombstoneAttr_Abort_Attr, abort) } else { // Old readers interpret every non-rowid/non-TS physical column as user - // data. Keep their commitTS-only layout and remove rollback holes before - // the batch can be flushed or published through logtail. + // data. Keep their commitTS-only layout, preserve the original physical + // row coordinates, and encode rollback holes as uncommitted rows so old + // readers filter them by commit timestamp. aborts := vector.MustFixedColWithTypeCheck[bool](abort.GetDownstreamVector()) for row, aborted := range aborts { if aborted { - inner.Delete(row) + commitTSVec.Update(row, txnif.UncommitTS, false) } } - inner.Compact() abort.Close() } if inner.Length() == 0 { diff --git a/pkg/vm/engine/tae/tables/object_test.go b/pkg/vm/engine/tae/tables/object_test.go index 7b9c80e00a10f..1081b361c6bea 100644 --- a/pkg/vm/engine/tae/tables/object_test.go +++ b/pkg/vm/engine/tae/tables/object_test.go @@ -29,6 +29,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/common" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/db/dbutils" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/iface/txnif" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/index/indexwrapper" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/tables/updates" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/testutils" @@ -270,8 +271,8 @@ func TestMemoryNodeRollbackHoleVisibilityAndWriteLayout(t *testing.T) { ) require.Equal(t, []bool{false, true, false}, aborts) - // During a rolling upgrade, the TN keeps the legacy layout and physically - // removes rollback holes so an old CN cannot expose them as user rows. + // During a rolling upgrade, the TN keeps the legacy layout and marks abort + // holes uncommitted so old readers hide them without shifting RowID offsets. rt.SetGlobalVariables(moruntime.MOProtocolVersion, defines.MORPCVersion9) legacyBatches := make(map[uint32]*containers.BatchWithVersion) err = mnode.getDataWindowOnWriteSchema( @@ -288,7 +289,13 @@ func TestMemoryNodeRollbackHoleVisibilityAndWriteLayout(t *testing.T) { require.NotContains(t, legacyBatch.Seqnums, uint16(objectio.SEQNUM_ABORT)) require.NotContains(t, legacyBatch.Nameidx, objectio.TombstoneAttr_Abort_Attr) require.Contains(t, legacyBatch.Seqnums, uint16(objectio.SEQNUM_COMMITTS)) - require.Equal(t, 2, legacyBatch.Length()) + require.Equal(t, 3, legacyBatch.Length()) + legacyCommitTS := vector.MustFixedColWithTypeCheck[types.TS]( + legacyBatch.GetVectorByName(objectio.TombstoneAttr_CommitTs_Attr).GetDownstreamVector(), + ) + require.Equal(t, types.BuildTS(1, 0), legacyCommitTS[0]) + require.Equal(t, txnif.UncommitTS, legacyCommitTS[1]) + require.Equal(t, types.BuildTS(3, 0), legacyCommitTS[2]) var output *containers.Batch err = mnode.Scan( From a718fc1a3b2ac2e07092961cce225b8ca17a31d4 Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Wed, 5 Aug 2026 14:11:54 +0800 Subject: [PATCH 10/11] fix: filter rollback rows in backup reads --- pkg/vm/engine/tae/blockio/read.go | 126 ++++++++++++++------- pkg/vm/engine/tae/blockio/read_test.go | 114 +++++++++++-------- pkg/vm/engine/tae/logtail/snapshot_test.go | 57 +++++++++- 3 files changed, 211 insertions(+), 86 deletions(-) diff --git a/pkg/vm/engine/tae/blockio/read.go b/pkg/vm/engine/tae/blockio/read.go index 1889692a21a8e..c4f0a917d2d99 100644 --- a/pkg/vm/engine/tae/blockio/read.go +++ b/pkg/vm/engine/tae/blockio/read.go @@ -37,7 +37,9 @@ import ( "github.com/matrixorigin/matrixone/pkg/vectorindex" "github.com/matrixorigin/matrixone/pkg/vectorindex/metric" "github.com/matrixorigin/matrixone/pkg/vm/engine" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/common" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/iface/txnif" "go.uber.org/zap" ) @@ -358,22 +360,21 @@ func BlockDataReadBackup( ts types.TS, fs fileservice.FileService, ) (loaded *batch.Batch, sortKey uint16, err error) { + location := info.MetaLocation() + requestedColumnCount := len(idxes) + commitPos, abortPos := -1, -1 if len(idxes) == 0 { - loaded, sortKey, err = ioutil.LoadOneBlock(ctx, fs, info.MetaLocation(), objectio.SchemaData) + var layout objectio.SpecialColumnLayout + loaded, sortKey, layout, err = ioutil.LoadOneBlockWithSpecialLayout( + ctx, fs, location, objectio.SchemaData, + ) + if pos, ok := layout.Resolve(objectio.SEQNUM_COMMITTS); ok { + commitPos = int(pos) + } + if pos, ok := layout.Resolve(objectio.SEQNUM_ABORT); ok { + abortPos = int(pos) + } } else { - loaded, sortKey, err = ioutil.LoadOneBlockWithIndex(ctx, fs, idxes, info.MetaLocation(), objectio.SchemaData) - } - // read block data from storage specified by meta location - if err != nil { - return - } - tombstones, err := ds.GetTombstones(ctx, &info.BlockID) - if err != nil { - return - } - defer tombstones.Release() - if !ts.IsEmpty() { - location := info.MetaLocation() objectMeta, metaErr := objectio.FastLoadObjectMeta(ctx, &location, false, fs) if metaErr != nil { err = metaErr @@ -381,41 +382,88 @@ func BlockDataReadBackup( } blockMeta := objectMeta.MustDataMeta().GetBlockMeta(uint32(location.ID())) layout := objectio.ResolveSpecialColumnLayout(blockMeta) - commitPos, ok := layout.Resolve(objectio.SEQNUM_COMMITTS) - if !ok { - err = moerr.NewInternalError(ctx, "backup object has no commit timestamp") - return + loadIdxes := slices.Clone(idxes) + if pos, ok := layout.Resolve(objectio.SEQNUM_COMMITTS); ok { + commitPos = slices.Index(loadIdxes, pos) + if commitPos < 0 { + commitPos = len(loadIdxes) + loadIdxes = append(loadIdxes, pos) + } } - commitTSs := vector.MustFixedColWithTypeCheck[types.TS](loaded.Vecs[commitPos]) - var aborts []bool - if abortPos, ok := layout.Resolve(objectio.SEQNUM_ABORT); ok { - abortVec := loaded.Vecs[abortPos] - if !abortVec.IsConstNull() { - aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + if pos, ok := layout.Resolve(objectio.SEQNUM_ABORT); ok { + abortPos = slices.Index(loadIdxes, pos) + if abortPos < 0 { + abortPos = len(loadIdxes) + loadIdxes = append(loadIdxes, pos) } } - visibleRows := make([]int64, 0, len(commitTSs)) - for row, commitTS := range commitTSs { - if commitTS.GT(&ts) || - (aborts != nil && aborts[row]) || - tombstones.Contains(uint64(row)) { - continue + loaded, sortKey, err = ioutil.LoadOneBlockWithIndex( + ctx, fs, loadIdxes, location, objectio.SchemaData, + ) + } + // read block data from storage specified by meta location + if err != nil { + return + } + defer func() { + if err != nil { + loaded.Clean(common.DebugAllocator) + loaded = nil + return + } + if requestedColumnCount > 0 { + for i := requestedColumnCount; i < len(loaded.Vecs); i++ { + loaded.Vecs[i].Free(common.DebugAllocator) + loaded.Vecs[i] = nil + } + loaded.Vecs = loaded.Vecs[:requestedColumnCount] + if len(loaded.Attrs) > requestedColumnCount { + loaded.Attrs = loaded.Attrs[:requestedColumnCount] } - visibleRows = append(visibleRows, int64(row)) } - if len(visibleRows) != len(commitTSs) { - loaded.Shrink(visibleRows, false) - logutil.Info("[BlockDataReadBackup]", - zap.String("ts", ts.ToString()), - zap.String("location", info.MetaLocation().String()), - zap.Int("rows", len(visibleRows))) + }() + tombstones, err := ds.GetTombstones(ctx, &info.BlockID) + if err != nil { + return + } + defer tombstones.Release() + if commitPos < 0 { + if !ts.IsEmpty() { + err = moerr.NewInternalError(ctx, "backup object has no commit timestamp") + return } - } else { rows := tombstones.ToI64Array(nil) if len(rows) > 0 { - logutil.Info("[BlockDataReadBackup Shrink]", zap.String("location", info.MetaLocation().String()), zap.Int("rows", len(rows))) + logutil.Info("[BlockDataReadBackup Shrink]", zap.String("location", location.String()), zap.Int("rows", len(rows))) loaded.Shrink(rows, true) } + return + } + + commitTSs := vector.MustFixedColWithTypeCheck[types.TS](loaded.Vecs[commitPos]) + var aborts []bool + if abortPos >= 0 { + abortVec := loaded.Vecs[abortPos] + if !abortVec.IsConstNull() { + aborts = vector.MustFixedColWithTypeCheck[bool](abortVec) + } + } + visibleRows := make([]int64, 0, len(commitTSs)) + for row, commitTS := range commitTSs { + if (!ts.IsEmpty() && commitTS.GT(&ts)) || + commitTS.Equal(&txnif.UncommitTS) || + (aborts != nil && aborts[row]) || + tombstones.Contains(uint64(row)) { + continue + } + visibleRows = append(visibleRows, int64(row)) + } + if len(visibleRows) != len(commitTSs) { + loaded.Shrink(visibleRows, false) + logutil.Info("[BlockDataReadBackup]", + zap.String("ts", ts.ToString()), + zap.String("location", location.String()), + zap.Int("rows", len(visibleRows))) } return } diff --git a/pkg/vm/engine/tae/blockio/read_test.go b/pkg/vm/engine/tae/blockio/read_test.go index e198a7d8a8c48..64a9b82ec0053 100644 --- a/pkg/vm/engine/tae/blockio/read_test.go +++ b/pkg/vm/engine/tae/blockio/read_test.go @@ -31,6 +31,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/vm/engine" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/common" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/iface/txnif" "github.com/stretchr/testify/require" ) @@ -425,53 +426,74 @@ func TestBlockDataReadInnerAppendableVisibility(t *testing.T) { } func TestBlockDataReadBackupCombinesAbortAndTombstoneMasks(t *testing.T) { - ctx := context.Background() - fs := testutil.NewSharedFS() - mp := mpool.MustNewZero() - defer mpool.DeleteMPool(mp) - - input := batch.NewWithSize(3) - input.Vecs[0] = vector.NewVec(types.T_int32.ToType()) - input.Vecs[1] = vector.NewVec(objectio.TSType) - input.Vecs[2] = vector.NewVec(types.T_bool.ToType()) - for row := range 4 { - require.NoError(t, vector.AppendFixed(input.Vecs[0], int32(row), false, mp)) - require.NoError(t, vector.AppendFixed(input.Vecs[1], types.BuildTS(1, 0), false, mp)) - require.NoError(t, vector.AppendFixed(input.Vecs[2], row == 1, false, mp)) + for _, test := range []struct { + name string + seqnums []uint16 + commitTS []types.TS + aborts []bool + }{ + { + name: "v10-abort-column", + seqnums: []uint16{0, objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT}, + commitTS: []types.TS{types.BuildTS(1, 0), types.BuildTS(1, 0), types.BuildTS(1, 0), types.BuildTS(1, 0)}, + aborts: []bool{false, true, false, false}, + }, + { + name: "v9-uncommitted-sentinel", + seqnums: []uint16{0, objectio.SEQNUM_COMMITTS}, + commitTS: []types.TS{types.BuildTS(1, 0), txnif.UncommitTS, types.BuildTS(1, 0), types.BuildTS(1, 0)}, + }, + } { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + fs := testutil.NewSharedFS() + mp := mpool.MustNewZero() + defer mpool.DeleteMPool(mp) + + input := batch.NewWithSize(len(test.seqnums)) + input.Vecs[0] = vector.NewVec(types.T_int32.ToType()) + input.Vecs[1] = vector.NewVec(objectio.TSType) + if len(test.aborts) > 0 { + input.Vecs[2] = vector.NewVec(types.T_bool.ToType()) + } + for row := range 4 { + require.NoError(t, vector.AppendFixed(input.Vecs[0], int32(row), false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[1], test.commitTS[row], false, mp)) + if len(test.aborts) > 0 { + require.NoError(t, vector.AppendFixed(input.Vecs[2], test.aborts[row], false, mp)) + } + } + input.SetRowCount(4) + writer := ioutil.ConstructWriter(0, test.seqnums, -1, false, false, fs) + writer.SetAppendable() + _, err := writer.WriteBatch(input) + require.NoError(t, err) + _, _, err = writer.Sync(ctx) + require.NoError(t, err) + stats := writer.GetObjectStats(objectio.WithAppendable()) + info := stats.ConstructBlockInfo(0) + input.Clean(mp) + + for _, idxes := range [][]uint16{nil, {0}} { + for _, cutoff := range []types.TS{types.BuildTS(2, 0), {}} { + loaded, _, err := BlockDataReadBackup( + ctx, + &info, + &blockReadTestDataSource{deleted: []uint64{2}}, + idxes, + cutoff, + fs, + ) + require.NoError(t, err) + if len(idxes) > 0 { + require.Len(t, loaded.Vecs, len(idxes)) + } + require.Equal(t, []int32{0, 3}, vector.MustFixedColWithTypeCheck[int32](loaded.Vecs[0])) + loaded.Clean(common.DebugAllocator) + } + } + }) } - input.SetRowCount(4) - writer := ioutil.ConstructWriter( - 0, - []uint16{0, objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT}, - -1, - false, - false, - fs, - ) - writer.SetAppendable() - _, err := writer.WriteBatch(input) - require.NoError(t, err) - _, _, err = writer.Sync(ctx) - require.NoError(t, err) - stats := writer.GetObjectStats(objectio.WithAppendable()) - info := stats.ConstructBlockInfo(0) - input.Clean(mp) - - loaded, _, err := BlockDataReadBackup( - ctx, - &info, - &blockReadTestDataSource{deleted: []uint64{2}}, - nil, - types.BuildTS(2, 0), - fs, - ) - require.NoError(t, err) - defer loaded.Clean(common.DebugAllocator) - require.Equal( - t, - []int32{0, 3}, - vector.MustFixedColWithTypeCheck[int32](loaded.Vecs[0]), - ) } func TestFillOutputBatchBySelectedRows(t *testing.T) { diff --git a/pkg/vm/engine/tae/logtail/snapshot_test.go b/pkg/vm/engine/tae/logtail/snapshot_test.go index a5f753cab0cc6..cb6f10514a2f1 100644 --- a/pkg/vm/engine/tae/logtail/snapshot_test.go +++ b/pkg/vm/engine/tae/logtail/snapshot_test.go @@ -15,10 +15,16 @@ package logtail import ( - "github.com/matrixorigin/matrixone/pkg/objectio" + "context" "testing" + "github.com/matrixorigin/matrixone/pkg/common/mpool" + "github.com/matrixorigin/matrixone/pkg/container/batch" "github.com/matrixorigin/matrixone/pkg/container/types" + "github.com/matrixorigin/matrixone/pkg/container/vector" + "github.com/matrixorigin/matrixone/pkg/objectio" + "github.com/matrixorigin/matrixone/pkg/objectio/ioutil" + "github.com/matrixorigin/matrixone/pkg/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -75,6 +81,55 @@ func TestSnapshotInfo(t *testing.T) { }) } +func TestSnapshotMetaGetSnapshotSkipsPersistedAborts(t *testing.T) { + ctx := context.Background() + fs := testutil.NewSharedFS() + mp := mpool.MustNewZero() + defer mpool.DeleteMPool(mp) + + input := batch.NewWithSize(len(snapshotSchemaTypes) + 2) + for i, typ := range snapshotSchemaTypes { + input.Vecs[i] = vector.NewVec(typ) + } + input.Vecs[len(snapshotSchemaTypes)] = vector.NewVec(objectio.TSType) + input.Vecs[len(snapshotSchemaTypes)+1] = vector.NewVec(types.T_bool.ToType()) + for row, ts := range []int64{100, 200} { + require.NoError(t, vector.AppendFixed(input.Vecs[ColSnapshotId], uint64(row+1), false, mp)) + require.NoError(t, vector.AppendBytes(input.Vecs[ColSName], []byte("snapshot"), false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[ColTS], ts, false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[ColLevel], types.Enum(SnapshotTypeCluster), false, mp)) + require.NoError(t, vector.AppendBytes(input.Vecs[ColAccountName], nil, false, mp)) + require.NoError(t, vector.AppendBytes(input.Vecs[ColDatabaseName], nil, false, mp)) + require.NoError(t, vector.AppendBytes(input.Vecs[ColTableName], nil, false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[ColObjId], uint64(0), false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[len(snapshotSchemaTypes)], types.BuildTS(1, 0), false, mp)) + require.NoError(t, vector.AppendFixed(input.Vecs[len(snapshotSchemaTypes)+1], row == 0, false, mp)) + } + input.SetRowCount(2) + seqnums := make([]uint16, 0, len(snapshotSchemaTypes)+2) + for i := range snapshotSchemaTypes { + seqnums = append(seqnums, uint16(i)) + } + seqnums = append(seqnums, objectio.SEQNUM_COMMITTS, objectio.SEQNUM_ABORT) + writer := ioutil.ConstructWriter(0, seqnums, -1, false, false, fs) + writer.SetAppendable() + _, err := writer.WriteBatch(input) + require.NoError(t, err) + _, _, err = writer.Sync(ctx) + require.NoError(t, err) + stats := writer.GetObjectStats(objectio.WithAppendable()) + input.Clean(mp) + + sm := NewSnapshotMeta() + const tableID = uint64(1) + sm.objects[tableID] = map[objectio.Segmentid]*objectInfo{ + stats.ObjectName().SegmentId(): {stats: stats}, + } + snapshots, err := sm.GetSnapshot(ctx, "test", fs, mp) + require.NoError(t, err) + require.Equal(t, []types.TS{types.BuildTS(200, 0)}, snapshots.cluster) +} + // TestAccountToTableSnapshots tests the core logic of snapshot distribution func TestAccountToTableSnapshots(t *testing.T) { // Create a mock SnapshotMeta From a89842070830fc5e8413145dc39581d438673a6c Mon Sep 17 00:00:00 2001 From: jiangxinmeng Date: Wed, 5 Aug 2026 17:39:37 +0800 Subject: [PATCH 11/11] fix: filter v9 rollback sentinels --- .../logtailreplay/v9_rollback_logtail_test.go | 81 ++++++++++++ pkg/vm/engine/tae/db/test/db_test.go | 72 ++++++++++ pkg/vm/engine/tae/logtail/tools.go | 25 ++-- pkg/vm/engine/tae/logtail/tools_test.go | 65 +++++++++ pkg/vm/engine/tae/tables/functions.go | 11 +- pkg/vm/engine/tae/tables/functions_test.go | 125 ++++++++++++------ 6 files changed, 330 insertions(+), 49 deletions(-) create mode 100644 pkg/vm/engine/disttae/logtailreplay/v9_rollback_logtail_test.go create mode 100644 pkg/vm/engine/tae/logtail/tools_test.go diff --git a/pkg/vm/engine/disttae/logtailreplay/v9_rollback_logtail_test.go b/pkg/vm/engine/disttae/logtailreplay/v9_rollback_logtail_test.go new file mode 100644 index 0000000000000..986ea5a7cdaf6 --- /dev/null +++ b/pkg/vm/engine/disttae/logtailreplay/v9_rollback_logtail_test.go @@ -0,0 +1,81 @@ +// Copyright 2021 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package logtailreplay_test + +import ( + "context" + "testing" + + "github.com/matrixorigin/matrixone/pkg/common/mpool" + "github.com/matrixorigin/matrixone/pkg/container/batch" + "github.com/matrixorigin/matrixone/pkg/container/types" + "github.com/matrixorigin/matrixone/pkg/objectio" + "github.com/matrixorigin/matrixone/pkg/vm/engine/disttae/logtailreplay" + "github.com/matrixorigin/matrixone/pkg/vm/engine/readutil" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/catalog" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/iface/txnif" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/logtail" + "github.com/stretchr/testify/require" +) + +func TestV9RollbackSentinelDoesNotReachPartitionState(t *testing.T) { + mp := mpool.MustNewZero() + defer mpool.DeleteMPool(mp) + + src := &containers.BatchWithVersion{ + Batch: containers.NewBatchWithCapacity(3), + Seqnums: []uint16{0, objectio.SEQNUM_ROWID, objectio.SEQNUM_COMMITTS}, + NextSeqnum: 1, + } + defer src.Close() + + primaryKeys := containers.MakeVector(types.T_int64.ToType(), mp) + rowIDs := containers.MakeVector(types.T_Rowid.ToType(), mp) + commitTSs := containers.MakeVector(types.T_TS.ToType(), mp) + var blockID types.Blockid + for row, key := range []int64{42, 7} { + primaryKeys.Append(key, false) + rowIDs.Append(types.NewRowid(&blockID, uint32(row)), false) + if row == 0 { + commitTSs.Append(txnif.UncommitTS, false) + } else { + commitTSs.Append(types.BuildTS(5, 0), false) + } + } + src.AddVector("pk", primaryKeys) + src.AddVector(catalog.PhyAddrColumnName, rowIDs) + src.AddVector(objectio.DefaultCommitTS_Attr, commitTSs) + + logtailBatch := logtail.DataChangeToLogtailBatch(src) + require.Equal(t, 1, logtailBatch.Length()) + protoBatch, err := batch.BatchToProtoBatch(containers.ToCNBatch(logtailBatch)) + require.NoError(t, err) + + state := logtailreplay.NewPartitionState("test", false, 42, false) + packer := types.NewPacker() + defer packer.Close() + state.HandleRowsInsert(context.Background(), protoBatch, 0, packer, mp) + + packer.Reset() + abortedKey := readutil.EncodePrimaryKey(int64(42), packer) + modified, _ := state.PKExistInMemBetween(types.TS{}, txnif.UncommitTS, [][]byte{abortedKey}) + require.False(t, modified) + + packer.Reset() + liveKey := readutil.EncodePrimaryKey(int64(7), packer) + modified, _ = state.PKExistInMemBetween(types.TS{}, txnif.UncommitTS, [][]byte{liveKey}) + require.True(t, modified) +} diff --git a/pkg/vm/engine/tae/db/test/db_test.go b/pkg/vm/engine/tae/db/test/db_test.go index e4831cc166d72..cbecf8a40f977 100644 --- a/pkg/vm/engine/tae/db/test/db_test.go +++ b/pkg/vm/engine/tae/db/test/db_test.go @@ -4062,6 +4062,8 @@ func TestV9FlushPreservesRowIDAcrossAbortHole(t *testing.T) { defer func() { if hadVersion { serviceRuntime.SetGlobalVariables(runtime.MOProtocolVersion, originalVersion) + } else { + serviceRuntime.CompareAndDeleteGlobalVariables(runtime.MOProtocolVersion, defines.MORPCVersion9) } }() @@ -4142,6 +4144,76 @@ func TestV9FlushPreservesRowIDAcrossAbortHole(t *testing.T) { require.NoError(t, snapshotTxn.Commit(ctx)) } +func TestV9PersistedTombstoneContainsSkipsRollback(t *testing.T) { + defer testutils.AfterTest(t)() + ctx := context.Background() + serviceRuntime := runtime.ServiceRuntime("") + originalVersion, hadVersion := serviceRuntime.GetGlobalVariables(runtime.MOProtocolVersion) + serviceRuntime.SetGlobalVariables(runtime.MOProtocolVersion, defines.MORPCVersion9) + defer func() { + if hadVersion { + serviceRuntime.SetGlobalVariables(runtime.MOProtocolVersion, originalVersion) + } else { + serviceRuntime.CompareAndDeleteGlobalVariables(runtime.MOProtocolVersion, defines.MORPCVersion9) + } + }() + + opts := config.WithLongScanAndCKPOpts(nil) + tae := testutil.NewTestEngine(ctx, ModuleName, t, opts) + defer tae.Close() + schema := catalog.MockSchemaAll(3, 2) + schema.Extra.BlockMaxRows = 10 + tae.BindSchema(schema) + rows := catalog.MockBatch(schema, 2) + defer rows.Close() + tae.CreateRelAndAppend(rows, true) + + abortTxn, abortRel := tae.GetRelation() + key := rows.Vecs[schema.GetSingleSortKeyIdx()].Get(0) + id, offset, err := abortRel.GetByFilter(ctx, handle.NewEQFilter(key)) + require.NoError(t, err) + target := objectio.NewRowid(&id.BlockID, offset) + require.NoError(t, abortRel.RangeDelete(id, offset, offset, handle.DT_Normal)) + require.NoError(t, abortTxn.PrePrepare(ctx)) + require.NoError(t, abortTxn.PreApplyCommit()) + require.NoError(t, abortTxn.Rollback(ctx)) + + deleteTxn, deleteRel := tae.GetRelation() + committedKey := rows.Vecs[schema.GetSingleSortKeyIdx()].Get(1) + committedID, committedOffset, err := deleteRel.GetByFilter(ctx, handle.NewEQFilter(committedKey)) + require.NoError(t, err) + require.NoError(t, deleteRel.RangeDelete(committedID, committedOffset, committedOffset, handle.DT_Normal)) + require.NoError(t, deleteTxn.Commit(ctx)) + + flushTxn, flushRel := tae.GetRelation() + tombstones := testutil.GetAllAppendableMetas(flushRel, true) + require.NotEmpty(t, tombstones) + task, err := jobs.NewFlushTableTailTask(nil, flushTxn, nil, tombstones, tae.Runtime) + require.NoError(t, err) + require.NoError(t, task.OnExec(ctx)) + require.NoError(t, flushTxn.Commit(ctx)) + + readTxn, readRel := tae.GetRelation() + var persisted *catalog.ObjectEntry + it := readRel.MakeObjectIt(true) + for it.Next() { + candidate := it.GetObject().GetMeta().(*catalog.ObjectEntry) + if !candidate.IsAppendable() && !candidate.HasDropCommitted() { + persisted = candidate + break + } + } + it.Close() + require.NotNil(t, persisted) + + rowIDs := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) + defer rowIDs.Close() + rowIDs.Append(target, false) + require.NoError(t, persisted.GetObjectData().Contains(ctx, readTxn, rowIDs, nil, common.DebugAllocator)) + require.False(t, rowIDs.IsNull(0), "a rolled-back v9 tombstone must not hide its data row") + require.NoError(t, readTxn.Commit(ctx)) +} + func TestIncrementalDedupIgnoresOldRowsInTNRewrite(t *testing.T) { defer testutils.AfterTest(t)() ctx := context.Background() diff --git a/pkg/vm/engine/tae/logtail/tools.go b/pkg/vm/engine/tae/logtail/tools.go index aabb1ebdd45f8..ed00978ac1f4d 100644 --- a/pkg/vm/engine/tae/logtail/tools.go +++ b/pkg/vm/engine/tae/logtail/tools.go @@ -29,6 +29,7 @@ import ( "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/catalog" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/common" "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/iface/txnif" "go.uber.org/zap/zapcore" ) @@ -176,19 +177,27 @@ func TombstoneChangeToLogtailBatch(src *containers.BatchWithVersion) *containers } func filterAbortedLogtailRows(src *containers.BatchWithVersion) { - abortPos := -1 + abortPos, commitTSPos := -1, -1 for i, seqnum := range src.Seqnums { - if seqnum == objectio.SEQNUM_ABORT { + switch seqnum { + case objectio.SEQNUM_ABORT: abortPos = i - break + case objectio.SEQNUM_COMMITTS: + commitTSPos = i } } + var aborts []bool if abortPos != -1 && !src.Vecs[abortPos].IsConstNull() { - aborts := vector.MustFixedColWithTypeCheck[bool](src.Vecs[abortPos].GetDownstreamVector()) - for row, aborted := range aborts { - if aborted { - src.Delete(row) - } + aborts = vector.MustFixedColWithTypeCheck[bool](src.Vecs[abortPos].GetDownstreamVector()) + } + var commitTSs []types.TS + if commitTSPos != -1 && !src.Vecs[commitTSPos].IsConstNull() { + commitTSs = vector.MustFixedColWithTypeCheck[types.TS](src.Vecs[commitTSPos].GetDownstreamVector()) + } + for row := 0; row < src.Length(); row++ { + if (row < len(aborts) && aborts[row]) || + (row < len(commitTSs) && commitTSs[row].Equal(&txnif.UncommitTS)) { + src.Delete(row) } } if src.HasDelete() { diff --git a/pkg/vm/engine/tae/logtail/tools_test.go b/pkg/vm/engine/tae/logtail/tools_test.go new file mode 100644 index 0000000000000..00d706e474386 --- /dev/null +++ b/pkg/vm/engine/tae/logtail/tools_test.go @@ -0,0 +1,65 @@ +// Copyright 2021 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package logtail + +import ( + "testing" + + "github.com/matrixorigin/matrixone/pkg/container/types" + "github.com/matrixorigin/matrixone/pkg/objectio" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/catalog" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/common" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/containers" + "github.com/matrixorigin/matrixone/pkg/vm/engine/tae/iface/txnif" + "github.com/stretchr/testify/require" +) + +func TestV9RollbackSentinelDoesNotReachTombstoneLogtail(t *testing.T) { + src := &containers.BatchWithVersion{ + Batch: containers.NewBatchWithCapacity(4), + Seqnums: []uint16{ + objectio.TombstoneAttr_Rowid_SeqNum, + objectio.TombstoneAttr_PK_SeqNum, + objectio.SEQNUM_ROWID, + objectio.SEQNUM_COMMITTS, + }, + NextSeqnum: 2, + } + defer src.Close() + + deletedRowIDs := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) + primaryKeys := containers.MakeVector(types.T_int64.ToType(), common.DefaultAllocator) + physicalRowIDs := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) + commitTSs := containers.MakeVector(types.T_TS.ToType(), common.DefaultAllocator) + var blockID types.Blockid + for row, key := range []int64{42, 7} { + deletedRowIDs.Append(types.NewRowid(&blockID, uint32(row+10)), false) + primaryKeys.Append(key, false) + physicalRowIDs.Append(types.NewRowid(&blockID, uint32(row)), false) + if row == 0 { + commitTSs.Append(txnif.UncommitTS, false) + } else { + commitTSs.Append(types.BuildTS(5, 0), false) + } + } + src.AddVector(objectio.TombstoneAttr_Rowid_Attr, deletedRowIDs) + src.AddVector(objectio.TombstoneAttr_PK_Attr, primaryKeys) + src.AddVector(catalog.PhyAddrColumnName, physicalRowIDs) + src.AddVector(objectio.TombstoneAttr_CommitTs_Attr, commitTSs) + + output := TombstoneChangeToLogtailBatch(src) + require.Equal(t, 1, output.Length()) + require.Equal(t, int64(7), output.GetVectorByName(objectio.TombstoneAttr_PK_Attr).Get(0)) +} diff --git a/pkg/vm/engine/tae/tables/functions.go b/pkg/vm/engine/tae/tables/functions.go index e89464a3be1fa..62471e482f299 100644 --- a/pkg/vm/engine/tae/tables/functions.go +++ b/pkg/vm/engine/tae/tables/functions.go @@ -349,6 +349,9 @@ func getDuplicatedRowIDABlkBytesFunc(args ...any) func([]byte, bool, int) error return nil } commitTS := vector.GetFixedAtNoTypeCheck[types.TS](tsVec.GetDownstreamVector(), row) + if commitTS.Equal(&txnif.UncommitTS) { + return nil + } startTS := txn.GetStartTS() // `from` is the first timestamp in the dedup window. Callers // advance their exclusive watermark with Next(), so equality @@ -411,6 +414,9 @@ func getDuplicatedRowIDABlkFuncFactory[T types.FixedSizeT](comp func(T, T) int) return nil } commitTS := tsVec.Get(row).(types.TS) + if commitTS.Equal(&txnif.UncommitTS) { + return nil + } // Keep the lower bound inclusive; see the varlen path. if commitTS.LT(&from) { return nil @@ -475,8 +481,11 @@ func containsABlkFuncFactory[T types.FixedSizeT](comp func(T, T) int) func(args if isAborted(abortVec, row) { continue } - rowIDs.Update(rowOffset, nil, true) commitTS := tsVec.Get(row).(types.TS) + if commitTS.Equal(&txnif.UncommitTS) { + continue + } + rowIDs.Update(rowOffset, nil, true) startTS := txn.GetStartTS() if commitTS.GT(&startTS) { ts, err := delsFn(v1, commitTS) diff --git a/pkg/vm/engine/tae/tables/functions_test.go b/pkg/vm/engine/tae/tables/functions_test.go index e04c6bc653334..f9598c0dc0360 100644 --- a/pkg/vm/engine/tae/tables/functions_test.go +++ b/pkg/vm/engine/tae/tables/functions_test.go @@ -72,41 +72,73 @@ func TestCommitTSLoaderCachesError(t *testing.T) { } func TestPersistedAppendableDedupSkipsAbortedRow(t *testing.T) { - data := containers.MakeVector(types.T_int64.ToType(), common.DefaultAllocator) - defer data.Close() - data.Append(int64(42), false) - keys := containers.MakeVector(types.T_int64.ToType(), common.DefaultAllocator) - defer keys.Close() - keys.Append(int64(42), false) - rowIDs := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) - defer rowIDs.Close() - rowIDs.Append(nil, true) - - commitTS := containers.MakeVector(types.T_TS.ToType(), common.DefaultAllocator) - commitTS.Append(types.BuildTS(5, 0), false) - abort := containers.MakeVector(types.T_bool.ToType(), common.DefaultAllocator) - abort.Append(true, false) - loader := &commitTSLoader{ - load: func() (containers.Vector, containers.Vector, error) { - return commitTS, abort, nil + for _, test := range []struct { + name string + commitTS types.TS + aborted bool + }{ + { + name: "v10-abort-column", + commitTS: types.BuildTS(5, 0), + aborted: true, + }, + { + name: "v9-uncommitted-sentinel", + commitTS: txnif.UncommitTS, }, + } { + for _, typ := range []types.Type{types.T_int64.ToType(), types.T_varchar.ToType()} { + t.Run(test.name+"/"+typ.String(), func(t *testing.T) { + data := containers.MakeVector(typ, common.DefaultAllocator) + defer data.Close() + keys := containers.MakeVector(typ, common.DefaultAllocator) + defer keys.Close() + if typ.Oid == types.T_int64 { + data.Append(int64(42), false) + keys.Append(int64(42), false) + } else { + data.Append([]byte("pk"), false) + keys.Append([]byte("pk"), false) + } + rowIDs := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) + defer rowIDs.Close() + rowIDs.Append(nil, true) + + commitTS := containers.MakeVector(types.T_TS.ToType(), common.DefaultAllocator) + commitTS.Append(test.commitTS, false) + var abort containers.Vector + if test.aborted { + abort = containers.MakeVector(types.T_bool.ToType(), common.DefaultAllocator) + abort.Append(true, false) + } else { + abort = containers.NewConstNullVector( + types.T_bool.ToType(), 1, common.DefaultAllocator, + ) + } + loader := &commitTSLoader{ + load: func() (containers.Vector, containers.Vector, error) { + return commitTS, abort, nil + }, + } + defer loader.close() + + txn := txnbase.MockTxnReaderWithStartTS(types.BuildTS(10, 0)) + op := containers.MakeForeachVectorOp( + keys.GetType().Oid, + getRowIDAlkFunctions, + data, + rowIDs, + types.Blockid{}, + loader, + txn, + types.TS{}, + types.MaxTs(), + ) + require.NoError(t, containers.ForeachVector(keys, op, nil)) + require.True(t, rowIDs.IsNull(0)) + }) + } } - defer loader.close() - - txn := txnbase.MockTxnReaderWithStartTS(types.BuildTS(10, 0)) - op := containers.MakeForeachVectorOp( - keys.GetType().Oid, - getRowIDAlkFunctions, - data, - rowIDs, - types.Blockid{}, - loader, - txn, - types.TS{}, - types.BuildTS(10, 0), - ) - require.NoError(t, containers.ForeachVector(keys, op, nil)) - require.True(t, rowIDs.IsNull(0)) } func TestPersistedTombstoneContainsSkipsAbortedMatches(t *testing.T) { @@ -114,15 +146,26 @@ func TestPersistedTombstoneContainsSkipsAbortedMatches(t *testing.T) { target := types.NewRowid(&blk, 7) txn := txnbase.MockTxnReaderWithStartTS(types.BuildTS(10, 0)) - check := func(aborts []bool) bool { + check := func(commitTSs []types.TS, aborts []bool) bool { persisted := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) defer persisted.Close() commitTS := containers.MakeVector(types.T_TS.ToType(), common.DefaultAllocator) - abortVec := containers.MakeVector(types.T_bool.ToType(), common.DefaultAllocator) - for _, aborted := range aborts { + var abortVec containers.Vector + if aborts == nil { + abortVec = containers.NewConstNullVector( + types.T_bool.ToType(), + len(commitTSs), + common.DefaultAllocator, + ) + } else { + abortVec = containers.MakeVector(types.T_bool.ToType(), common.DefaultAllocator) + } + for row, ts := range commitTSs { persisted.Append(target, false) - commitTS.Append(types.BuildTS(5, 0), false) - abortVec.Append(aborted, false) + commitTS.Append(ts, false) + if aborts != nil { + abortVec.Append(aborts[row], false) + } } keys := containers.MakeVector(types.T_Rowid.ToType(), common.DefaultAllocator) defer keys.Close() @@ -146,8 +189,10 @@ func TestPersistedTombstoneContainsSkipsAbortedMatches(t *testing.T) { return keys.IsNull(0) } - require.True(t, check([]bool{true, false}), "a live physical match must delete the data row") - require.False(t, check([]bool{true, true}), "all-aborted physical matches must not delete the data row") + committed := types.BuildTS(5, 0) + require.True(t, check([]types.TS{committed, committed}, []bool{true, false}), "a live physical match must delete the data row") + require.False(t, check([]types.TS{committed, committed}, []bool{true, true}), "all-aborted physical matches must not delete the data row") + require.False(t, check([]types.TS{txnif.UncommitTS}, nil), "a v9 rollback sentinel must not delete the data row") } func TestMissingCommitTSFollowsDedupPolicy(t *testing.T) {