Repository: matrixorigin/matrixone
Pull request #5465 (2022-10-08 16:07:17)

remove dummy code
## What type of PR is this?

- [ ] API-change
- [ ] BUG
- [ ] Improvement
- [ ] Documentation
- [ ] Feature
- [ ] Test and CI
- [x] Code Refactoring

## Which issue(s) this PR fixes:

issue #

## What this PR does / why we need it:
1. Remove dummy code
2. Small code refactor


Patch:
diff --git a/pkg/vm/engine/tae/catalog/metabase.go b/pkg/vm/engine/tae/catalog/metabase.go
index a43d0b3a0..0bf707488 100644
--- a/pkg/vm/engine/tae/catalog/metabase.go
+++ b/pkg/vm/engine/tae/catalog/metabase.go
@@ -222,6 +222,12 @@ func (be *MetaBaseEntry) DoCompre(voe BaseEntry) int {
 }
 
 func (be *MetaBaseEntry) HasDropped() bool {
+	be.RLock()
+	defer be.RUnlock()
+	return be.HasDroppedLocked()
+}
+
+func (be *MetaBaseEntry) HasDroppedLocked() bool {
 	node := be.GetCommittedNode()
 	if node == nil {
 		return false
@@ -271,7 +277,7 @@ func (be *MetaBaseEntry) DropEntryLocked(txnCtx txnif.TxnReader) (isNewNode bool
 	if err != nil {
 		return
 	}
-	if be.HasDropped() {
+	if be.HasDroppedLocked() {
 		return false, moerr.NewNotFound()
 	}
 	isNewNode, err = be.DeleteLocked(txnCtx)
diff --git a/pkg/vm/engine/tae/catalog/segment.go b/pkg/vm/engine/tae/catalog/segment.go
index a5c60a4d8..0887deb9b 100644
--- a/pkg/vm/engine/tae/catalog/segment.go
+++ b/pkg/vm/engine/tae/catalog/segment.go
@@ -222,9 +222,7 @@ func (entry *SegmentEntry) LastAppendableBlock() (blk *BlockEntry) {
 	it := entry.MakeBlockIt(false)
 	for it.Valid() {
 		itBlk := it.Get().GetPayload()
-		itBlk.RLock()
 		dropped := itBlk.HasDropped()
-		itBlk.RUnlock()
 		if itBlk.IsAppendable() && !dropped {
 			blk = itBlk
 			break
diff --git a/pkg/vm/engine/tae/catalog/table.go b/pkg/vm/engine/tae/catalog/table.go
index be3dc2f01..f4672816b 100644
--- a/pkg/vm/engine/tae/catalog/table.go
+++ b/pkg/vm/engine/tae/catalog/table.go
@@ -255,9 +255,7 @@ func (entry *TableEntry) LastAppendableSegmemt() (seg *SegmentEntry) {
 	it := entry.MakeSegmentIt(false)
 	for it.Valid() {
 		itSeg := it.Get().GetPayload()
-		itSeg.RLock()
 		dropped := itSeg.HasDropped()
-		itSeg.RUnlock()
 		if itSeg.IsAppendable() && !dropped {
 			seg = itSeg
 			break
diff --git a/pkg/vm/engine/tae/iface/data/block.go b/pkg/vm/engine/tae/iface/data/block.go
index 930538bfb..a2725db50 100644
--- a/pkg/vm/engine/tae/iface/data/block.go
+++ b/pkg/vm/engine/tae/iface/data/block.go
@@ -66,7 +66,7 @@ type Block interface {
 	GetRowsOnReplay() uint64
 	GetID() *common.ID
 	IsAppendable() bool
-	SetNotAppendable()
+	FreezeAppend()
 
 	Rows(txn txnif.AsyncTxn, coarse bool) int
 	GetColumnDataByName(txn txnif.AsyncTxn, attr string, buffer *bytes.Buffer) (*model.ColumnView, error)
@@ -90,7 +90,6 @@ type Block interface {
 
 	SetMaxCheckpointTS(ts types.TS)
 	GetMaxCheckpointTS() types.TS
-	GetMaxVisibleTS() types.TS
 
 	CheckpointWALClosure(endTs types.TS) tasks.FuncT
 	Destroy() error
diff --git a/pkg/vm/engine/tae/tables/block.go b/pkg/vm/engine/tae/tables/block.go
index aa4076119..bec6dabcb 100644
--- a/pkg/vm/engine/tae/tables/block.go
+++ b/pkg/vm/engine/tae/tables/block.go
@@ -51,14 +51,6 @@ func init() {
 	ImmutMemAllocator = stl.NewSimpleAllocator()
 }
 
-type BlockState int8
-
-// BlockState is the state of the data block, which is different from the state of the mate
-const (
-	BS_Appendable BlockState = iota
-	BS_NotAppendable
-)
-
 // The initial state of the block when scoring
 type statBlock struct {
 	rows      uint32
@@ -69,19 +61,19 @@ type dataBlock struct {
 	common.RefHelper
 	*sync.RWMutex
 	common.ClosedState
-	meta       *catalog.BlockEntry
-	node       *appendableNode
-	file       file.Block
-	colObjects map[int]file.ColumnBlock
-	bufMgr     base.INodeManager
-	scheduler  tasks.TaskScheduler
-	indexes    map[int]indexwrapper.Index
-	pkIndex    indexwrapper.Index // a shortcut, nil if no pk column
-	mvcc       *updates.MVCCHandle
-	score      *statBlock
-	ckpTs      atomic.Value
-	prefix     []byte
-	state      BlockState
+	meta         *catalog.BlockEntry
+	node         *appendableNode
+	file         file.Block
+	colObjects   map[int]file.ColumnBlock
+	bufMgr       base.INodeManager
+	scheduler    tasks.TaskScheduler
+	indexes      map[int]indexwrapper.Index
+	pkIndex      indexwrapper.Index // a shortcut, nil if no pk column
+	mvcc         *updates.MVCCHandle
+	score        *statBlock
+	ckpTs        atomic.Value
+	prefix       []byte
+	appendFrozen bool
 }
 
 func newBlock(meta *catalog.BlockEntry, segFile file.Segment, bufMgr base.INodeManager, scheduler tasks.TaskScheduler) *dataBlock {
@@ -128,7 +120,7 @@ func newBlock(meta *catalog.BlockEntry, segFile file.Segment, bufMgr base.INodeM
 		bufMgr:     bufMgr,
 		prefix:     meta.MakeKey(),
 	}
-	ts := block.ReadTS()
+	block.SetMaxCheckpointTS(types.TS{})
 	block.mvcc.SetAppendListener(block.OnApplyAppend)
 	if meta.IsAppendable() {
 		block.mvcc.SetDeletesListener(block.ABlkApplyDelete)
@@ -146,14 +138,11 @@ func newBlock(meta *catalog.BlockEntry, segFile file.Segment, bufMgr base.INodeM
 				block.indexes[colDef.Idx] = indexwrapper.NewMutableIndex(colDef.Type)
 			}
 		}
-		block.state = BS_Appendable
 	} else {
 		block.mvcc.SetDeletesListener(block.BlkApplyDelete)
 		// if this block is created to do compact or merge, no need to new index
 		// if this block is loaded from storage, ReplayIndex will create index
 	}
-	block.mvcc.SetMaxVisible(ts)
-	block.ckpTs.Store(ts)
 	if meta.GetMetaLoc() != "" {
 		if err := block.ReplayIndex(); err != nil {
 			panic(err)
@@ -165,19 +154,18 @@ func newBlock(meta *catalog.BlockEntry, segFile file.Segment, bufMgr base.INodeM
 	return block
 }
 
-func (blk *dataBlock) ReadTS() (ts types.TS)        { return }
 func (blk *dataBlock) GetMeta() any                 { return blk.meta }
 func (blk *dataBlock) GetBufMgr() base.INodeManager { return blk.bufMgr }
-func (blk *dataBlock) SetNotAppendable() {
+func (blk *dataBlock) FreezeAppend() {
 	blk.Lock()
 	defer blk.Unlock()
-	blk.state = BS_NotAppendable
+	blk.appendFrozen = true
 }
 
-func (blk *dataBlock) GetBlockState() BlockState {
+func (blk *dataBlock) IsAppendFrozen() bool {
 	blk.RLock()
 	defer blk.RUnlock()
-	return blk.state
+	return blk.appendFrozen
 }
 
 func (blk *dataBlock) SetMaxCheckpointTS(ts types.TS) {
@@ -189,10 +177,6 @@ func (blk *dataBlock) GetMaxCheckpointTS() types.TS {
 	return ts
 }
 
-func (blk *dataBlock) GetMaxVisibleTS() types.TS {
-	return blk.mvcc.LoadMaxVisible()
-}
-
 func (blk *dataBlock) FreeData() {
 	blk.Lock()
 	defer blk.Unlock()
@@ -247,31 +231,45 @@ func (blk *dataBlock) GetBlockFile() file.Block {
 
 func (blk *dataBlock) GetID() *common.ID { return blk.meta.AsCommonID() }
 
-func (blk *dataBlock) RunCalibration() int {
-	if blk.meta.IsAppendable() && blk.Rows(nil, true) == int(blk.meta.GetSchema().BlockMaxRows) {
-		blk.meta.RLock()
-		if blk.meta.HasDropped() {
-			blk.meta.RUnlock()
-			return 0
-		}
-		blk.meta.RUnlock()
-		return 100
-	}
-	return blk.estimateRawScore()
+func (blk *dataBlock) RunCalibration() (score int) {
+	score, _ = blk.estimateRawScore()
+	return
 }
 
-func (blk *dataBlock) estimateRawScore() int {
-	if blk.Rows(nil, true) == int(blk.meta.GetSchema().BlockMaxRows) && blk.meta.IsAppendable() {
+func (blk *dataBlock) estimateABlkRawScore() (score int) {
+	// Max row appended
+	rows := blk.Rows(nil, true)
+	if rows == int(blk.meta.GetSchema().BlockMaxRows) {
 		return 100
 	}
 
-	if blk.mvcc.GetChangeNodeCnt() == 0 && !blk.meta.IsAppendable() {
-		return 0
-	} else if blk.mvcc.GetChangeNodeCnt() == 0 && blk.meta.IsAppendable() &&
-		blk.mvcc.LoadMaxVisible().LessEq(blk.GetMaxCheckpointTS()) {
-		return 0
+	if blk.mvcc.GetChangeNodeCnt() == 0 && rows == 0 {
+		// No deletes or append found
+		score = 0
+	} else {
+		// Any deletes or append
+		score = 1
 	}
-	return 1
+	return
+}
+
+func (blk *dataBlock) estimateRawScore() (score int, dropped bool) {
+	if blk.meta.HasDropped() {
+		dropped = true
+		return
+	}
+	if blk.meta.IsAppendable() {
+		score = blk.estimateABlkRawScore()
+		return
+	}
+	if blk.mvcc.GetChangeNodeCnt() == 0 {
+		// No deletes found
+		score = 0
+	} else {
+		// Any delete
+		score = 1
+	}
+	return
 }
 
 func (blk *dataBlock) MutationInfo() string {
@@ -299,17 +297,10 @@ func (blk *dataBlock) MutationInfo() string {
 }
 
 func (blk *dataBlock) EstimateScore(interval int64) int {
-	if blk.meta.IsAppendable() && blk.Rows(nil, true) == int(blk.meta.GetSchema().BlockMaxRows) {
-		blk.meta.RLock()
-		if blk.meta.HasDropped() {
-			blk.meta.RUnlock()
-			return 0
-		}
-		blk.meta.RUnlock()
-		return 100
+	score, dropped := blk.estimateRawScore()
+	if dropped {
+		return 0
 	}
-
-	score := blk.estimateRawScore()
 	if score > 1 {
 		return score
 	}
@@ -340,7 +331,7 @@ func (blk *dataBlock) BuildCompactionTaskFactory() (
 	scopes []common.ID,
 	err error) {
 	// If the conditions are met, immediately modify the data block status to NotAppendable
-	blk.SetNotAppendable()
+	blk.FreezeAppend()
 	blk.meta.RLock()
 	dropped := blk.meta.IsDroppedCommitted()
 	inTxn := blk.meta.IsCreating()
@@ -365,7 +356,7 @@ func (blk *dataBlock) IsAppendable() bool {
 		return false
 	}
 
-	if blk.GetBlockState() == BS_NotAppendable {
+	if blk.IsAppendFrozen() {
 		return false
 	}
 
diff --git a/pkg/vm/engine/tae/tables/handle.go b/pkg/vm/engine/tae/tables/handle.go
index b68773055..7c2fb7f4c 100644
--- a/pkg/vm/engine/tae/tables/handle.go
+++ b/pkg/vm/engine/tae/tables/handle.go
@@ -85,17 +85,13 @@ func (h *tableHandle) GetAppender() (appender data.BlockAppender, err error) {
 			panic(err)
 		}
 	}
-	h.block.meta.RLock()
 	dropped := h.block.meta.HasDropped()
-	h.block.meta.RUnlock()
 	if !h.appender.IsAppendable() || !h.block.IsAppendable() || dropped {
 		return h.ThrowAppenderAndErr()
 	}
 	h.block.Ref()
 	// Similar to optimistic locking
-	h.block.meta.RLock()
 	dropped = h.block.meta.HasDropped()
-	h.block.meta.RUnlock()
 	if !h.appender.IsAppendable() || !h.block.IsAppendable() || dropped {
 		h.block.Unref()
 		return h.ThrowAppenderAndErr()
diff --git a/pkg/vm/engine/tae/tables/updates/append.go b/pkg/vm/engine/tae/tables/updates/append.go
index 51bf57e07..57e2d1736 100644
--- a/pkg/vm/engine/tae/tables/updates/append.go
+++ b/pkg/vm/engine/tae/tables/updates/append.go
@@ -21,7 +21,6 @@ import (
 
 	"github.com/matrixorigin/matrixone/pkg/container/types"
 
-	"github.com/matrixorigin/matrixone/pkg/logutil"
 	"github.com/matrixorigin/matrixone/pkg/vm/engine/tae/common"
 	"github.com/matrixorigin/matrixone/pkg/vm/engine/tae/iface/txnif"
 	"github.com/matrixorigin/matrixone/pkg/vm/engine/tae/txn/txnbase"
@@ -151,10 +150,6 @@ func (node *AppendNode) ApplyCommit(index *wal.Index) error {
 		panic("AppendNode | ApplyCommit | LogicErr")
 	}
 	node.TxnMVCCNode.ApplyCommit(index)
-	if node.mvcc != nil {
-		logutil.Debugf("Set MaxCommitTS=%v, MaxVisibleRow=%d", node.GetEnd(), node.GetMaxRow())
-		node.mvcc.SetMaxVisible(node.GetEnd())
-	}
 	// logutil.Infof("Apply1Index %s TS=%d", index.String(), n.commitTs)
 	listener := node.mvcc.GetAppendListener()
 	if listener == nil {
diff --git a/pkg/vm/engine/tae/tables/updates/colupdate.go b/pkg/vm/engine/tae/tables/updates/colupdate.go
index 416ef161e..23f214555 100644
--- a/pkg/vm/engine/tae/tables/updates/colupdate.go
+++ b/pkg/vm/engine/tae/tables/updates/colupdate.go
@@ -357,7 +357,6 @@ func (node *ColumnUpdateNode) ApplyCommit(index *wal.Index) (err error) {
 	}
 	node.txn = nil
 	node.logIndex = index
-	node.chain.mvcc.SetMaxVisible(node.commitTs)
 	node.chain.mvcc.IncChangeNodeCnt()
 	return
 }
diff --git a/pkg/vm/engine/tae/tables/updates/delete.go b/pkg/vm/engine/tae/tables/updates/delete.go
index b1a426134..90868a2b2 100644
--- a/pkg/vm/engine/tae/tables/updates/delete.go
+++ b/pkg/vm/engine/tae/tables/updates/delete.go
@@ -167,14 +167,10 @@ func (node *DeleteNode) PrepareCommit() (err error) {
 
 func (node *DeleteNode) ApplyCommit(index *wal.Index) (err error) {
 	node.chain.mvcc.Lock()
-	var ts types.TS
-	ts, err = node.TxnMVCCNode.ApplyCommit(index)
+	_, err = node.TxnMVCCNode.ApplyCommit(index)
 	if err != nil {
 		return
 	}
-	if node.chain.mvcc != nil {
-		node.chain.mvcc.SetMaxVisible(ts)
-	}
 	node.chain.AddDeleteCnt(uint32(node.mask.GetCardinality()))
 	node.chain.mvcc.IncChangeNodeCnt()
 	node.chain.mvcc.Unlock()
diff --git a/pkg/vm/engine/tae/tables/updates/mvcc.go b/pkg/vm/engine/tae/tables/updates/mvcc.go
index 6c90a073e..a8b741af6 100644
--- a/pkg/vm/engine/tae/tables/updates/mvcc.go
+++ b/pkg/vm/engine/tae/tables/updates/mvcc.go
@@ -36,9 +36,7 @@ type MVCCHandle struct {
 	*sync.RWMutex
 	columns         map[uint16]*ColumnChain
 	deletes         *DeleteChain
-	holes           *roaring.Bitmap
 	meta            *catalog.BlockEntry
-	maxVisible      atomic.Value
 	appends         *txnbase.MVCCSlice
 	changes         uint32
 	deletesListener func(uint64, common.RowGen, types.TS) error
@@ -85,32 +83,10 @@ func (n *MVCCHandle) HasActiveAppendNode() bool {
 	return !n.appends.IsCommitted()
 }
 
-func (n *MVCCHandle) AddHoles(start, end int) {
-	if n.holes != nil {
-		n.holes = roaring.New()
-	}
-	n.holes.AddRange(uint64(start), uint64(end))
-}
-
-func (n *MVCCHandle) HasHole() bool {
-	return n.holes != nil && !n.holes.IsEmpty()
-}
-
-func (n *MVCCHandle) HoleCnt() int {
-	if !n.HasHole() {
-		return 0
-	}
-	return int(n.holes.GetCardinality())
-}
-
 func (n *MVCCHandle) IncChangeNodeCnt() {
 	atomic.AddUint32(&n.changes, uint32(1))
 }
 
-func (n *MVCCHandle) ResetChangeNodeCnt() {
-	atomic.StoreUint32(&n.changes, uint32(0))
-}
-
 func (n *MVCCHandle) GetChangeNodeCnt() uint32 {
 	return atomic.LoadUint32(&n.changes)
 }
@@ -123,15 +99,6 @@ func (n *MVCCHandle) GetDeleteCnt() uint32 {
 	return n.deletes.GetDeleteCnt()
 }
 
-func (n *MVCCHandle) SetMaxVisible(ts types.TS) {
-	n.maxVisible.Store(ts)
-}
-
-func (n *MVCCHandle) LoadMaxVisible() types.TS {
-	ts := n.maxVisible.Load().(types.TS)
-	return ts
-}
-
 func (n *MVCCHandle) GetID() *common.ID { return n.meta.AsCommonID() }
 
 func (n *MVCCHandle) StringLocked() string {
@@ -166,7 +133,6 @@ func (n *MVCCHandle) CreateDeleteNode(txn txnif.AsyncTxn, deleteType handle.Dele
 
 func (n *MVCCHandle) OnReplayDeleteNode(deleteNode txnif.DeleteNode) {
 	n.deletes.OnReplayNode(deleteNode.(*DeleteNode))
-	n.TrySetMaxVisible(deleteNode.(*DeleteNode).GetCommitTSLocked())
 }
 
 func (n *MVCCHandle) CreateUpdateNode(colIdx uint16, txn txnif.AsyncTxn) txnif.UpdateNode {
@@ -206,12 +172,6 @@ func (n *MVCCHandle) GetDeleteChain() *DeleteChain {
 func (n *MVCCHandle) OnReplayAppendNode(an *AppendNode) {
 	an.mvcc = n
 	n.appends.InsertNode(an)
-	n.TrySetMaxVisible(an.GetCommitTS())
-}
-func (n *MVCCHandle) TrySetMaxVisible(ts types.TS) {
-	if ts.Greater(n.maxVisible.Load().(types.TS)) {
-		n.maxVisible.Store(ts)
-	}
 }
 func (n *MVCCHandle) AddAppendNodeLocked(
 	txn txnif.AsyncTxn,
diff --git a/pkg/vm/engine/tae/tables/updates/updatechain.go b/pkg/vm/engine/tae/tables/updates/updatechain.go
index 782091f5b..124ca038e 100644
--- a/pkg/vm/engine/tae/tables/updates/updatechain.go
+++ b/pkg/vm/engine/tae/tables/updates/updatechain.go
@@ -91,7 +91,6 @@ func (chain *ColumnChain) TryUpdateNodeLocked(row uint32, v any, n txnif.UpdateN
 
 func (chain *ColumnChain) OnReplayUpdateNode(updateNode txnif.UpdateNode) {
 	updateNode.(*ColumnUpdateNode).AttachTo(chain)
-	chain.mvcc.TrySetMaxVisible(updateNode.(*ColumnUpdateNode).commitTs)
 	mask := updateNode.GetMask()
 	vals := updateNode.GetValues()
 	iterator := mask.Iterator()


Test patch:
diff --git a/pkg/vm/engine/tae/db/tables_test.go b/pkg/vm/engine/tae/db/tables_test.go
index f1ca04c10..859fc2c42 100644
--- a/pkg/vm/engine/tae/db/tables_test.go
+++ b/pkg/vm/engine/tae/db/tables_test.go
@@ -765,7 +765,7 @@ func TestCompaction2(t *testing.T) {
 	schema.BlockMaxRows = 21
 	schema.SegmentMaxBlocks = 4
 	cnt := uint32(3)
-	rows := schema.BlockMaxRows / 3 * cnt
+	rows := schema.BlockMaxRows
 	bat := catalog.MockBatch(schema, int(rows))
 	defer bat.Close()
 	bats := bat.Split(int(cnt))
@@ -777,22 +777,7 @@ func TestCompaction2(t *testing.T) {
 		assert.Nil(t, err)
 		assert.Nil(t, txn.Commit())
 	}
-	{
-		txn, _ := db.StartTxn(nil)
-		database, _ := txn.GetDatabase("db")
-		rel, _ := database.GetRelationByName(schema.Name)
-		it := rel.MakeBlockIt()
-		for it.Valid() {
-			blk := it.GetBlock()
-			view, _ := blk.GetColumnDataById(3, nil)
-			assert.NotNil(t, view)
-			view.Close()
-			assert.True(t, blk.GetMeta().(*catalog.BlockEntry).IsAppendable())
-			assert.True(t, blk.GetMeta().(*catalog.BlockEntry).GetBlockData().IsAppendable())
-			it.Next()
-		}
-		time.Sleep(400 * time.Millisecond)
-	}
+	time.Sleep(400 * time.Millisecond)
 	{
 		txn, _ := db.StartTxn(nil)
 		database, _ := txn.GetDatabase("db")
@@ -808,15 +793,6 @@ func TestCompaction2(t *testing.T) {
 			it.Next()
 		}
 	}
-	{
-		txn, _ := db.StartTxn(nil)
-		database, _ := txn.GetDatabase("db")
-		rel, _ := database.GetRelationByName(schema.Name)
-		err := rel.Append(bats[1])
-		assert.Nil(t, err)
-		assert.Nil(t, txn.Commit())
-		time.Sleep(400 * time.Millisecond)
-	}
 	{
 		txn, _ := db.StartTxn(nil)
 		database, _ := txn.GetDatabase("db")