Repository: vanus-labs/vanus
Pull request #603 (2023-06-01 03:24:34)

fix(store): unexpected compaction after restart
<!--

Thank you for contributing to Vanus!

PR Title Format: 

feat/fix/refactor/docs/...: what's changed

Note: see https://github.com/vanus-labs/vanus/blob/main/CONTRIBUTING.md to know details about title format

-->

### What problem does this PR solve?

<!--

Please create an issue first to describe the problem

There MUST be one line starting with "Issue Number:  ".

-->

Issue Number: None

Fix #437 is incomplete.

### Problem Summary

### What is changed and how does it work?

### Check List

<!-- At least one of them must be included. -->

#### Tests

- [ ] Unit test
- [ ] Integration test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No code


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs) Report
> Merging [#603](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs) (bb3a1c9) into [main](https://app.codecov.io/gh/vanus-labs/vanus/commit/ee76217182ba60efbcbb62fd374b8e75127d5133?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs) (ee76217) will **decrease** coverage by `2.52%`.
> The diff coverage is `46.82%`.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/vanus-labs/vanus/pull/603/graphs/tree.svg?width=650&height=150&src=pr&token=RSXSIMEY4V&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs)](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs)

```diff
@@            Coverage Diff             @@
##             main     #603      +/-   ##
==========================================
- Coverage   57.07%   54.55%   -2.52%     
==========================================
  Files         190      211      +21     
  Lines       15191    17048    +1857     
==========================================
+ Hits         8670     9301     +631     
- Misses       5884     7042    +1158     
- Partials      637      705      +68     
```


| [Impacted Files](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs) | Coverage Δ | |
|---|---|---|
| [internal/controller/trigger/storage/storage.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvY29udHJvbGxlci90cmlnZ2VyL3N0b3JhZ2Uvc3RvcmFnZS5nbw==) | `0.00% <ø> (ø)` | |
| [...ernal/controller/trigger/validation/subscripton.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvY29udHJvbGxlci90cmlnZ2VyL3ZhbGlkYXRpb24vc3Vic2NyaXB0b24uZ28=) | `66.12% <ø> (-3.17%)` | :arrow_down: |
| [internal/controller/trigger/worker/manager.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvY29udHJvbGxlci90cmlnZ2VyL3dvcmtlci9tYW5hZ2VyLmdv) | `85.76% <ø> (+0.83%)` | :arrow_up: |
| [internal/controller/trigger/worker/policy.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvY29udHJvbGxlci90cmlnZ2VyL3dvcmtlci9wb2xpY3kuZ28=) | `100.00% <ø> (ø)` | |
| [internal/controller/trigger/worker/scheduler.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvY29udHJvbGxlci90cmlnZ2VyL3dvcmtlci9zY2hlZHVsZXIuZ28=) | `71.01% <ø> (+1.01%)` | :arrow_up: |
| [internal/controller/trigger/worker/worker.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvY29udHJvbGxlci90cmlnZ2VyL3dvcmtlci93b3JrZXIuZ28=) | `78.45% <ø> (-1.55%)` | :arrow_down: |
| [internal/gateway/config.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvZ2F0ZXdheS9jb25maWcuZ28=) | `60.60% <ø> (-0.51%)` | :arrow_down: |
| [internal/gateway/gateway.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvZ2F0ZXdheS9nYXRld2F5Lmdv) | `57.64% <ø> (-10.03%)` | :arrow_down: |
| [internal/gateway/proxy/auth.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvZ2F0ZXdheS9wcm94eS9hdXRoLmdv) | `0.00% <ø> (ø)` | |
| [internal/gateway/proxy/deadletter.go](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs#diff-aW50ZXJuYWwvZ2F0ZXdheS9wcm94eS9kZWFkbGV0dGVyLmdv) | `0.00% <ø> (ø)` | |
| ... and [70 more](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs) | |

... and [58 files with indirect coverage changes](https://app.codecov.io/gh/vanus-labs/vanus/pull/603/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs)

------

[Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs). Last update [43db43b...bb3a1c9](https://app.codecov.io/gh/vanus-labs/vanus/pull/603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vanus-labs).

</details>

Patch:
diff --git a/internal/store/raft/storage/compaction.go b/internal/store/raft/storage/compaction.go
index 281d08b..18b3560 100644
--- a/internal/store/raft/storage/compaction.go
+++ b/internal/store/raft/storage/compaction.go
@@ -255,23 +255,14 @@ func (w *WAL) removeNode(ctx context.Context, nodeID vanus.ID) error {
 }
 
 func (w *WAL) recoverNode(nodeID vanus.ID, offset int64) {
-	task := adminTask{
-		nodeID: nodeID,
-		offset: offset,
-	}
-
-	w.closeMu.RLock()
-	select {
-	case <-w.closeC:
-	default:
-		w.compactC <- task.recoverNode
+	w.nodes[nodeID] = true
+	if offset != 0 {
+		w.barrier.Set(offset, nodeID)
 	}
-	w.closeMu.RUnlock()
 }
 
 type adminTask struct {
 	nodeID vanus.ID
-	offset int64
 	ch     chan<- error
 }
 
@@ -309,13 +300,6 @@ func (t *adminTask) removeNode(w *WAL, cCtx *compactContext) {
 	})
 }
 
-func (t *adminTask) recoverNode(w *WAL, _ *compactContext) {
-	w.nodes[t.nodeID] = true
-	if t.offset != 0 {
-		w.barrier.Set(t.offset, t.nodeID)
-	}
-}
-
 type compactInfo struct {
 	index, term uint64
 }
diff --git a/internal/store/raft/storage/recovery.go b/internal/store/raft/storage/recovery.go
index bdf2091..bac7798 100644
--- a/internal/store/raft/storage/recovery.go
+++ b/internal/store/raft/storage/recovery.go
@@ -123,7 +123,7 @@ func Recover(
 	if err != nil {
 		return nil, nil, err
 	}
-	wal2 := newWAL(wal, stateStore)
+	wal2 := newWAL(wal, stateStore, false)
 
 	// convert, and set wal
 	storages := make(map[vanus.ID]*Storage, len(sb.storages))
@@ -140,6 +140,9 @@ func Recover(
 		storages[nodeID] = storage
 	}
 
+	// Start compaction after recover nodes.
+	wal2.startCompaction()
+
 	return storages, wal2, nil
 }
 
diff --git a/internal/store/raft/storage/wal.go b/internal/store/raft/storage/wal.go
index 413ed4c..24f0268 100644
--- a/internal/store/raft/storage/wal.go
+++ b/internal/store/raft/storage/wal.go
@@ -45,7 +45,7 @@ type WAL struct {
 	doneC   chan struct{}
 }
 
-func newWAL(wal *walog.WAL, stateStore *meta.SyncStore) *WAL {
+func newWAL(wal *walog.WAL, stateStore *meta.SyncStore, startCompaction bool) *WAL {
 	w := &WAL{
 		WAL:        wal,
 		stateStore: stateStore,
@@ -56,11 +56,17 @@ func newWAL(wal *walog.WAL, stateStore *meta.SyncStore) *WAL {
 		doneC:      make(chan struct{}),
 	}
 
-	go w.runCompact()
+	if startCompaction {
+		w.startCompaction()
+	}
 
 	return w
 }
 
+func (w *WAL) startCompaction() {
+	go w.runCompact()
+}
+
 func (w *WAL) Close() {
 	w.WAL.Close()
 	go func() {


Test patch:
diff --git a/internal/store/raft/storage/compaction_test.go b/internal/store/raft/storage/compaction_test.go
index fbb17c1..7961d53 100644
--- a/internal/store/raft/storage/compaction_test.go
+++ b/internal/store/raft/storage/compaction_test.go
@@ -53,7 +53,7 @@ func TestStorage_Compact(t *testing.T) {
 
 		rawWAL, err := walog.Open(ctx, walDir, walog.WithFileSize(int64(fileSize)))
 		So(err, ShouldBeNil)
-		wal := newWAL(rawWAL, stateStore)
+		wal := newWAL(rawWAL, stateStore, true)
 
 		s1, _ := NewStorage(ctx, nodeID1, wal, stateStore, hintStore, nil)
 		s2, _ := NewStorage(ctx, nodeID2, wal, stateStore, hintStore, nil)
diff --git a/internal/store/raft/storage/snapshot_storage_test.go b/internal/store/raft/storage/snapshot_storage_test.go
index 0541cc5..7693e1f 100644
--- a/internal/store/raft/storage/snapshot_storage_test.go
+++ b/internal/store/raft/storage/snapshot_storage_test.go
@@ -61,7 +61,7 @@ func TestStorage_SnapshotStorage(t *testing.T) {
 
 		rawWAL, err := walog.Open(ctx, walDir, walog.WithFileSize(int64(fileSize)))
 		So(err, ShouldBeNil)
-		wal := newWAL(rawWAL, stateStore)
+		wal := newWAL(rawWAL, stateStore, true)
 		defer wal.Close()
 
 		snapOp := NewMockSnapshotOperator(ctrl)
diff --git a/internal/store/raft/storage/storage_test.go b/internal/store/raft/storage/storage_test.go
index a7bb4ab..182184a 100644
--- a/internal/store/raft/storage/storage_test.go
+++ b/internal/store/raft/storage/storage_test.go
@@ -86,7 +86,7 @@ func TestStorage(t *testing.T) {
 		Convey("create raft log", func() {
 			rawWAL, err := walog.Open(ctx, raftDir, walog.WithFileSize(int64(fileSize)))
 			So(err, ShouldBeNil)
-			wal := newWAL(rawWAL, stateStore)
+			wal := newWAL(rawWAL, stateStore, true)
 			defer func() {
 				wal.Close()
 				wal.Wait()