feat: optimize route switching when freeze segment #321

hwjiangkai · 2022-12-05T05:07:33Z

What problem does this PR solve?

Optimize the routing logic for segment full

Issue Number: close #xxx

Problem Summary

What is changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

codecov · 2022-12-05T05:27:22Z

Codecov Report

Merging #321 (8c06b70) into main (09f3748) will decrease coverage by 3.64%.
The diff coverage is 54.38%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #321      +/-   ##
==========================================
- Coverage   62.72%   59.08%   -3.64%     
==========================================
  Files         133      175      +42     
  Lines       12128    14474    +2346     
==========================================
+ Hits         7607     8552     +945     
- Misses       3977     5306    +1329     
- Partials      544      616      +72

Impacted Files	Coverage Δ
internal/controller/eventbus/server/instance.go	`72.04% <ø> (ø)`
internal/controller/trigger/storage/secret.go	`48.48% <ø> (ø)`
...nternal/controller/trigger/storage/subscription.go	`54.71% <ø> (ø)`
...ernal/controller/trigger/storage/trigger_worker.go	`63.41% <ø> (ø)`
...rnal/primitive/interceptor/errinterceptor/error.go	`0.00% <0.00%> (ø)`
.../primitive/interceptor/memberinterceptor/member.go	`0.00% <0.00%> (ø)`
internal/primitive/vanus/id.go	`19.68% <0.00%> (-0.65%)`	⬇️
internal/raft/log/recovery.go	`52.27% <ø> (ø)`
internal/raft/log/wal.go	`100.00% <ø> (ø)`
internal/store/block/entry.go	`70.96% <ø> (ø)`
... and 159 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1157347...8c06b70. Read the comment docs.

internal/store/block/raw.go

internal/store/block/raft/appender.go

internal/store/segment/replica.go

internal/store/vsb/block_open_test.go

internal/store/vsb/block_append_test.go

ifplusor · 2022-12-12T02:03:28Z

internal/store/vsb/block.go

@@ -106,14 +106,15 @@ func (b *vsBlock) Delete(context.Context) error {
 }

 func (b *vsBlock) status() block.Statistics {
-	return b.stat(b.makeSnapshot())
+	m, indexes := b.makeSnapshot()
+	return b.stat(m, indexes, block.StateWorking)


pass state with m.archived.

m.archived has been deleted, and the segment status cannot be rolled back

meta.archived

client/pkg/eventlog/eventlog_impl.go

Signed-off-by: jyjiangkai <[email protected]>

client/pkg/eventbus/eventbus.go

ifplusor · 2022-12-21T09:11:15Z

client/pkg/eventlog/eventlog_impl.go

+	if l.logWriter != nil {
+		return l.logWriter


maybe access l.logWriter is not safe.

ifplusor · 2022-12-22T05:32:23Z

internal/store/vsb/block_append_test.go

@@ -297,7 +288,6 @@ func TestVSBlock_Append(t *testing.T) {
 		So(archived, ShouldBeTrue)

 		stat = b.status()
-		So(stat.Archived, ShouldBeTrue)


check state

ifplusor · 2022-12-22T05:32:38Z

internal/store/vsb/block_open_test.go

@@ -104,7 +105,6 @@ func TestVSBlock_Open(t *testing.T) {

 		stat := b.status()
 		So(stat.Capacity, ShouldEqual, vsbtest.EntrySize0+vsbtest.EntrySize1)
-		So(stat.Archived, ShouldBeTrue)


check state

ifplusor · 2022-12-22T05:32:42Z

internal/store/vsb/block_open_test.go

@@ -141,7 +141,6 @@ func TestVSBlock_Open(t *testing.T) {

 		stat := b.status()
 		So(stat.Capacity, ShouldEqual, vsbtest.EntrySize0+vsbtest.EntrySize1)
-		So(stat.Archived, ShouldBeFalse)


check state

internal/trigger/server_test.go

ifplusor · 2022-12-22T05:35:45Z

internal/store/block/raw.go

+	StateArchived  = State("archived")
+)
+
+func (s State) ToSegmentState() string {


don't place this at here.

ifplusor · 2022-12-22T05:44:24Z

client/pkg/eventlog/eventlog_impl.go

+
+	segmentNum := len(l.readableSegments)
+	n := sort.Search(segmentNum, func(i int) bool {
+		return l.readableSegments[uint64(i)].EndOffset() > offset


the key of readableSegments is not a index

Signed-off-by: jyjiangkai <[email protected]>

ifplusor · 2023-02-06T07:36:38Z

internal/store/vsb/block_append.go

-	return seqs, frag, actx.size(b.dataOffset) >= b.capacity, nil
+	full := actx.size(b.dataOffset) >= b.capacity
+	if full && b.lis != nil {
+		m, indexes := makeSnapshot(b.actx, b.indexes)


don't use b.actx and b.indexes in PrepareAppend()

ifplusor · 2023-02-06T07:39:30Z

internal/store/segment/server.go

+
+				s.replicas.Range(func(key, value interface{}) bool {
+					b, _ := value.(replica)
+					if b.appender.Status().Leader.Equals(vanus.ID(segment.LeaderBlockId)) {


use LeaderBlockId is unsafe, because it may be not equal to info.leader from info.term.

ifplusor · 2023-02-06T07:55:16Z

internal/store/vsb/block.go

+	state := block.StateWorking
+	m, indexes := b.makeSnapshot()
+	if m.archived {
+		state = block.StateArchiving


StateArchiving does not match the semantics of archived.

And the value of state needs to persist to disk.

ifplusor · 2023-02-06T08:08:54Z

client/pkg/eventbus/eventbus.go

@@ -216,6 +219,8 @@ func (b *eventbus) ListLog(ctx context.Context, opts ...api.LogOption) ([]api.Ev
 	}

 	if op.Policy.AccessMode() == api.ReadOnly {
+		b.readableMu.RLock()
+		defer b.readableMu.RUnlock()
 		if len(b.readableLogs) == 0 {
 			b.refreshReadableLogs(ctx)


unlock when refresh route info.

ifplusor · 2023-02-06T08:09:14Z

client/pkg/eventbus/eventbus.go

@@ -225,6 +230,8 @@ func (b *eventbus) ListLog(ctx context.Context, opts ...api.LogOption) ([]api.Ev
 		}
 		return eventlogs, nil
 	} else if op.Policy.AccessMode() == api.ReadWrite {
+		b.writableMu.RLock()
+		defer b.writableMu.RUnlock()
 		if len(b.writableLogs) == 0 {
 			b.refreshWritableLogs(ctx)


unlock when refresh route info

ifplusor · 2023-02-06T08:13:17Z

client/pkg/eventlog/eventlog_impl.go

+	l.logWriter = nil
+	l.logReader = nil


why do not call Close()?

ifplusor · 2023-02-06T08:16:06Z

client/pkg/eventbus/eventbus.go

@@ -185,19 +184,23 @@ func (b *eventbus) GetLog(ctx context.Context, logID uint64, opts ...api.LogOpti
 	}

 	if op.Policy.AccessMode() == api.ReadOnly {
+		b.readableMu.RLock()
+		defer b.readableMu.RUnlock()
 		if len(b.readableLogs) == 0 {
 			b.refreshReadableLogs(ctx)


unlock when refresh route info.

ifplusor · 2023-02-06T08:16:11Z

client/pkg/eventbus/eventbus.go

 		}
 		return nil, errors.ErrResourceNotFound.WithMessage("eventlog not found")
 	} else if op.Policy.AccessMode() == api.ReadWrite {
+		b.writableMu.RLock()
+		defer b.writableMu.RUnlock()
 		if len(b.writableLogs) == 0 {
 			b.refreshWritableLogs(ctx)


unlock when refresh route info.

ifplusor · 2023-02-06T08:22:12Z

client/pkg/eventlog/eventlog_impl.go

+	// LastEntryStime
+	// time.UnixMilli
+	tailSeg := fetchTailSegment(ctx, segs)
+	if tailSeg.firstEventBornAt.Before(t) {


use firstEventBornAt is unreasonable.

Signed-off-by: jyjiangkai <[email protected]>

hwjiangkai requested review from ifplusor and wenfengwang as code owners December 5, 2022 05:07

feat: Optimize the routing logic for segment full

9c505cc

hwjiangkai force-pushed the segment branch from 53722e2 to 9c505cc Compare December 5, 2022 05:23

ifplusor reviewed Dec 9, 2022

View reviewed changes

internal/store/block/raw.go Outdated Show resolved Hide resolved

ifplusor requested changes Dec 9, 2022

View reviewed changes

internal/store/block/raw.go Outdated Show resolved Hide resolved

internal/store/block/raw.go Outdated Show resolved Hide resolved

internal/store/block/raft/appender.go Show resolved Hide resolved

hwjiangkai force-pushed the segment branch 2 times, most recently from b4563e9 to 8b132b7 Compare December 9, 2022 08:07

fix review

074e7f9

hwjiangkai force-pushed the segment branch from 8b132b7 to 074e7f9 Compare December 9, 2022 08:09

ifplusor requested changes Dec 20, 2022

View reviewed changes

feat: fix review comments

9351af1

Signed-off-by: jyjiangkai <[email protected]>

hwjiangkai requested a review from xdlbdy as a code owner December 21, 2022 06:08

Merge branch 'main' of github.com:linkall-labs/vanus into segment

88c1479

Signed-off-by: jyjiangkai <[email protected]>

hwjiangkai force-pushed the segment branch from e7fb772 to 88c1479 Compare December 21, 2022 06:15

ifplusor requested changes Dec 22, 2022

View reviewed changes

ifplusor reviewed Dec 22, 2022

View reviewed changes

feat: fix review comments

5b3b241

Signed-off-by: jyjiangkai <[email protected]>

hwjiangkai force-pushed the segment branch from ea22811 to 2bf7967 Compare February 6, 2023 03:40

Merge branch 'main' of github.com:linkall-labs/vanus into segment

8b9f873

Signed-off-by: jyjiangkai <[email protected]>

hwjiangkai force-pushed the segment branch from 2bf7967 to 8b9f873 Compare February 6, 2023 05:00

ifplusor requested changes Feb 6, 2023

View reviewed changes

ifplusor reviewed Feb 6, 2023

View reviewed changes

ifplusor requested changes Feb 6, 2023

View reviewed changes

hwjiangkai force-pushed the segment branch 2 times, most recently from af4e196 to f4afed5 Compare February 6, 2023 12:52

fix review comments

8c06b70

Signed-off-by: jyjiangkai <[email protected]>

hwjiangkai force-pushed the segment branch from f4afed5 to 8c06b70 Compare February 6, 2023 12:56

ifplusor changed the title ~~feat: Optimize the routing logic for segment full~~ feat: optimize route switching when freeze segment Feb 10, 2023

ifplusor self-assigned this Feb 14, 2023

ifplusor marked this pull request as draft February 25, 2023 11:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: optimize route switching when freeze segment #321

feat: optimize route switching when freeze segment #321

hwjiangkai commented Dec 5, 2022

codecov bot commented Dec 5, 2022 •

edited

Loading

ifplusor Dec 12, 2022

hwjiangkai Dec 21, 2022

ifplusor Dec 22, 2022

ifplusor Dec 21, 2022

ifplusor Dec 22, 2022

ifplusor Dec 22, 2022

ifplusor Dec 22, 2022

ifplusor Dec 22, 2022

ifplusor Dec 22, 2022

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

ifplusor Feb 6, 2023

feat: optimize route switching when freeze segment #321

Are you sure you want to change the base?

feat: optimize route switching when freeze segment #321

Conversation

hwjiangkai commented Dec 5, 2022

What problem does this PR solve?

Problem Summary

What is changed and how does it work?

Check List

Tests

codecov bot commented Dec 5, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 5, 2022 •

edited

Loading