Merge pull request #373 from fuweid/fix-213

fix data race on tx.Stats

.github/workflows/tests.yaml

@@ -1,18 +1,89 @@
 name: Tests
 on: [push, pull_request]
 jobs:
-  test:
+  test-linux:
     strategy:
+      fail-fast: false
       matrix:
-        os: [ubuntu-latest, windows-latest]
-    runs-on: ${{ matrix.os }}
+        target:
+        - linux-amd64-unit-test-4-cpu
+        - linux-amd64-unit-test-4-cpu-freelist-hashmap-race
+        - linux-amd64-unit-test-2-cpu-freelist-array-short-race
+    runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
     - uses: actions/setup-go@v3
       with:
         go-version: "1.17.13"
     - run: make fmt
-    - run: make race
-    - run: make test
+    - env:
+        TARGET: ${{ matrix.target }}
+      run: |
+        case "${TARGET}" in
+          linux-amd64-unit-test-4-cpu)
+            CPU=4 make test
+            ;;
+          linux-amd64-unit-test-4-cpu-freelist-hashmap-race)
+            CPU=4 ENABLE_RACE=true make test-freelist-hashmap
+            ;;
+          linux-amd64-unit-test-2-cpu-freelist-array-short-race)
+            CPU=2 ENABLE_RACE=true EXTRA_TESTFLAGS="-short" make test-freelist-array
+            ;;
+          *)
+            echo "Failed to find target"
+            exit 1
+            ;;
+        esac
     - name: golangci-lint
       uses: golangci/golangci-lint-action@0ad9a0988b3973e851ab0a07adf248ec2e100376 # v3.3.1
+
+  test-windows:
+    strategy:
+      fail-fast: false
+      matrix:
+        target:
+        - windows-amd64-unit-test-4-cpu
+        # FIXME(fuweid):
+        #
+        # The windows will throws the following error when enable race.
+        # We skip it until we have solution.
+        #
+        #   ThreadSanitizer failed to allocate 0x000200000000 (8589934592) bytes at 0x0400c0000000 (error code: 1455)
+        #
+        #- windows-amd64-unit-test-4-cpu-race
+    runs-on: windows-latest
+    steps:
+    - uses: actions/checkout@v3
+    - uses: actions/setup-go@v3
+      with:
+        go-version: "1.17.13"
+    - run: make fmt
+    - env:
+        TARGET: ${{ matrix.target }}
+      run: |
+        case "${TARGET}" in
+          windows-amd64-unit-test-4-cpu)
+            CPU=4 make test
+            ;;
+          *)
+            echo "Failed to find target"
+            exit 1
+            ;;
+        esac
+      shell: bash
+    - name: golangci-lint
+      uses: golangci/golangci-lint-action@0ad9a0988b3973e851ab0a07adf248ec2e100376 # v3.3.1
+
+  coverage:
+    needs: ["test-linux", "test-windows"]
+    strategy:
+      matrix:
+        os: [ubuntu-latest, windows-latest]
+    runs-on: ${{ matrix.os }}
+    steps:
+    - uses: actions/checkout@v3
+    - uses: actions/setup-go@v3
+      with:
+        go-version: "1.17.13"
+    - run: make coverage
+

.gitignore

@@ -3,5 +3,6 @@
 *.swp
 /bin/
 cover.out
+cover-*.out
 /.idea
 *.iml

Makefile

@@ -2,24 +2,42 @@ BRANCH=`git rev-parse --abbrev-ref HEAD`
 COMMIT=`git rev-parse --short HEAD`
 GOLDFLAGS="-X main.branch $(BRANCH) -X main.commit $(COMMIT)"
 
-race:
-	@TEST_FREELIST_TYPE=hashmap go test -v -race -test.run="TestSimulate_(100op|1000op)"
-	@echo "array freelist test"
-	@TEST_FREELIST_TYPE=array go test -v -race -test.run="TestSimulate_(100op|1000op)"
+TESTFLAGS_RACE=-race=false
+ifdef ENABLE_RACE
+	TESTFLAGS_RACE=-race=true
+endif
+
+TESTFLAGS_CPU=
+ifdef CPU
+	TESTFLAGS_CPU=-cpu=$(CPU)
+endif
+TESTFLAGS = $(TESTFLAGS_RACE) $(TESTFLAGS_CPU) $(EXTRA_TESTFLAGS)
 
 fmt:
 	!(gofmt -l -s -d $(shell find . -name \*.go) | grep '[a-z]')
 
 lint:
 	golangci-lint run ./...
 
-test:
-	TEST_FREELIST_TYPE=hashmap go test -timeout 30m -v -coverprofile cover.out -covermode atomic
-	TEST_FREELIST_TYPE=hashmap go test -v ./cmd/bbolt
+test: test-freelist-hashmap test-freelist-array
+
+test-freelist-hashmap:
+	@echo "hashmap freelist test"
+	TEST_FREELIST_TYPE=hashmap go test -v ${TESTFLAGS} -timeout 30m
+	TEST_FREELIST_TYPE=hashmap go test -v ${TESTFLAGS} ./cmd/bbolt
 
+test-freelist-array:
 	@echo "array freelist test"
+	TEST_FREELIST_TYPE=array go test -v ${TESTFLAGS} -timeout 30m
+	TEST_FREELIST_TYPE=array go test -v ${TESTFLAGS} ./cmd/bbolt
 
-	@TEST_FREELIST_TYPE=array go test -timeout 30m -v -coverprofile cover.out -covermode atomic
-	@TEST_FREELIST_TYPE=array go test -v ./cmd/bbolt
+coverage:
+	@echo "hashmap freelist test"
+	TEST_FREELIST_TYPE=hashmap go test -v -timeout 30m \
+		-coverprofile cover-freelist-hashmap.out -covermode atomic
+
+	@echo "array freelist test"
+	TEST_FREELIST_TYPE=array go test -v -timeout 30m \
+		-coverprofile cover-freelist-array.out -covermode atomic
 
-.PHONY: race fmt test lint
+.PHONY: fmt test test-freelist-hashmap test-freelist-array lint

allocate_test.go

@@ -18,14 +18,16 @@ func TestTx_allocatePageStats(t *testing.T) {
 		pages: make(map[pgid]*page),
 	}
 
-	prePageCnt := tx.Stats().PageCount
+	txStats := tx.Stats()
+	prePageCnt := txStats.GetPageCount()
 	allocateCnt := f.free_count()
 
 	if _, err := tx.allocate(allocateCnt); err != nil {
 		t.Fatal(err)
 	}
 
-	if tx.Stats().PageCount != prePageCnt+allocateCnt {
-		t.Errorf("Allocated %d but got %d page in stats", allocateCnt, tx.Stats().PageCount)
+	txStats = tx.Stats()
+	if txStats.GetPageCount() != prePageCnt+int64(allocateCnt) {
+		t.Errorf("Allocated %d but got %d page in stats", allocateCnt, txStats.GetPageCount())
 	}
 }

bucket.go

@@ -81,7 +81,7 @@ func (b *Bucket) Writable() bool {
 // Do not use a cursor after the transaction is closed.
 func (b *Bucket) Cursor() *Cursor {
 	// Update transaction statistics.
-	b.tx.stats.CursorCount++
+	b.tx.stats.IncCursorCount(1)
 
 	// Allocate and return a cursor.
 	return &Cursor{
@@ -681,7 +681,7 @@ func (b *Bucket) node(pgid pgid, parent *node) *node {
 	b.nodes[pgid] = n
 
 	// Update statistics.
-	b.tx.stats.NodeCount++
+	b.tx.stats.IncNodeCount(1)
 
 	return n
 }

db_test.go

@@ -682,7 +682,9 @@ func TestDB_Concurrent_WriteTo(t *testing.T) {
 			panic(err)
 		}
 		f.Close()
-		snap := btesting.MustOpenDBWithOption(t, f.Name(), o)
+
+		copyOpt := *o
+		snap := btesting.MustOpenDBWithOption(t, f.Name(), &copyOpt)
 		defer snap.MustClose()
 		snap.MustCheck()
 	}
@@ -1036,8 +1038,8 @@ func TestDB_Stats(t *testing.T) {
 	}
 
 	stats := db.Stats()
-	if stats.TxStats.PageCount != 2 {
-		t.Fatalf("unexpected TxStats.PageCount: %d", stats.TxStats.PageCount)
+	if stats.TxStats.GetPageCount() != 2 {
+		t.Fatalf("unexpected TxStats.PageCount: %d", stats.TxStats.GetPageCount())
 	} else if stats.FreePageN != 0 {
 		t.Fatalf("unexpected FreePageN != 0: %d", stats.FreePageN)
 	} else if stats.PendingPageN != 2 {
@@ -1120,8 +1122,8 @@ func TestDBStats_Sub(t *testing.T) {
 	b.TxStats.PageCount = 10
 	b.FreePageN = 14
 	diff := b.Sub(&a)
-	if diff.TxStats.PageCount != 7 {
-		t.Fatalf("unexpected TxStats.PageCount: %d", diff.TxStats.PageCount)
+	if diff.TxStats.GetPageCount() != 7 {
+		t.Fatalf("unexpected TxStats.PageCount: %d", diff.TxStats.GetPageCount())
 	}
 
 	// free page stats are copied from the receiver and not subtracted

internal/btesting/btesting.go

@@ -189,14 +189,14 @@ func (db *DB) CopyTempFile() {
 func (db *DB) PrintStats() {
 	var stats = db.Stats()
 	fmt.Printf("[db] %-20s %-20s %-20s\n",
-		fmt.Sprintf("pg(%d/%d)", stats.TxStats.PageCount, stats.TxStats.PageAlloc),
-		fmt.Sprintf("cur(%d)", stats.TxStats.CursorCount),
-		fmt.Sprintf("node(%d/%d)", stats.TxStats.NodeCount, stats.TxStats.NodeDeref),
+		fmt.Sprintf("pg(%d/%d)", stats.TxStats.GetPageCount(), stats.TxStats.GetPageAlloc()),
+		fmt.Sprintf("cur(%d)", stats.TxStats.GetCursorCount()),
+		fmt.Sprintf("node(%d/%d)", stats.TxStats.GetNodeCount(), stats.TxStats.GetNodeDeref()),
 	)
 	fmt.Printf("     %-20s %-20s %-20s\n",
-		fmt.Sprintf("rebal(%d/%v)", stats.TxStats.Rebalance, truncDuration(stats.TxStats.RebalanceTime)),
-		fmt.Sprintf("spill(%d/%v)", stats.TxStats.Spill, truncDuration(stats.TxStats.SpillTime)),
-		fmt.Sprintf("w(%d/%v)", stats.TxStats.Write, truncDuration(stats.TxStats.WriteTime)),
+		fmt.Sprintf("rebal(%d/%v)", stats.TxStats.GetRebalance(), truncDuration(stats.TxStats.GetRebalanceTime())),
+		fmt.Sprintf("spill(%d/%v)", stats.TxStats.GetSpill(), truncDuration(stats.TxStats.GetSpillTime())),
+		fmt.Sprintf("w(%d/%v)", stats.TxStats.GetWrite(), truncDuration(stats.TxStats.GetWriteTime())),
 	)
 }
 

node.go

@@ -304,7 +304,7 @@ func (n *node) splitTwo(pageSize uintptr) (*node, *node) {
 	n.inodes = n.inodes[:splitIndex]
 
 	// Update the statistics.
-	n.bucket.tx.stats.Split++
+	n.bucket.tx.stats.IncSplit(1)
 
 	return n, next
 }
@@ -391,7 +391,7 @@ func (n *node) spill() error {
 		}
 
 		// Update the statistics.
-		tx.stats.Spill++
+		tx.stats.IncSpill(1)
 	}
 
 	// If the root node split and created a new root then we need to spill that
@@ -413,7 +413,7 @@ func (n *node) rebalance() {
 	n.unbalanced = false
 
 	// Update statistics.
-	n.bucket.tx.stats.Rebalance++
+	n.bucket.tx.stats.IncRebalance(1)
 
 	// Ignore if node is above threshold (25%) and has enough keys.
 	var threshold = n.bucket.tx.db.pageSize / 4
@@ -547,7 +547,7 @@ func (n *node) dereference() {
 	}
 
 	// Update statistics.
-	n.bucket.tx.stats.NodeDeref++
+	n.bucket.tx.stats.IncNodeDeref(1)
 }
 
 // free adds the node's underlying page to the freelist.