From 69ec6fe58e334a6b6d46bcf9db61c0ff6155fe73 Mon Sep 17 00:00:00 2001 From: vladopajic Date: Mon, 22 Jan 2024 11:23:54 +0100 Subject: [PATCH 1/7] add common hurdles page (#70) --- README.md | 4 +++- docs/common_hurdles.md | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 docs/common_hurdles.md diff --git a/README.md b/README.md index 3f844a9..825cb50 100644 --- a/README.md +++ b/README.md @@ -116,10 +116,12 @@ func (w *consumeWorker) DoWork(ctx actor.Context) actor.WorkerStatus { - [super](https://github.com/vladopajic/go-super-actor) is addon abstraction which aims to unify testing of actor's and worker's business logic. - [commence](https://github.com/vladopajic/go-actor-commence) is addon which gives mechanism for waiting on actors execution to commence. -## Best practices +## Pro tips To enhance the code quality of projects that heavily rely on the actor model and utilize the `go-actor` library, it's recommended to adhere to [best practices](./docs/best_practices.md). +Reading about [common hurdles](./docs/common_hurdles.md), where the most frequent issues are documented, is also advisable. + ## Design decisions Design decisions are documented [here](./docs/design_decisions.md). diff --git a/docs/common_hurdles.md b/docs/common_hurdles.md new file mode 100644 index 0000000..b818acd --- /dev/null +++ b/docs/common_hurdles.md @@ -0,0 +1,35 @@ +# Common hurdles + +## Nothing is happening here + +One of the most common hurdles is the case where actors are not started. This is usually experienced when the program is initiated, but nothing is happening in the segment of the program that we are working on and aiming to test. Whenever we experience that nothing is happening, we should double-check that all actors are started. + +**Mailbox is not started** + +Never forget that `actor.Mailbox` is also an actor, and it needs to be started. + +## Default case is undesirable + +Workers should always block when there isn't anything to work on; therefore, their `select` statements shouldn't have a `default` case. If workers do not block, they will simply waste computation cycles. + +Example: +```go +func (w *consumeWorker) DoWork(ctx actor.Context) actor.WorkerStatus { + select { + case <-ctx.Done(): + return actor.WorkerEnd + + case num := <-w.mailbox.ReceiveC(): + fmt.Printf("consumed %d \t(worker %d)\n", num, w.id) + + return actor.WorkerContinue + + default: // <----------------------- warning: this worker will never block! default case is undesirable! + return actor.WorkerContinue + } +} +``` + +--- + +// Your contribution is valuable; if you have encountered any challenges, please share your experiences. \ No newline at end of file From e69e92495e9485768345838bbe0605b50fdb2af7 Mon Sep 17 00:00:00 2001 From: vladopajic Date: Wed, 7 Feb 2024 16:39:52 +0100 Subject: [PATCH 2/7] go version bump to v1.22 (#71) --- .github/workflows/lint.yml | 7 +++---- Makefile | 2 +- actor/actor_test.go | 14 ++++++++++---- actor/combine_test.go | 6 +++--- actor/context_test.go | 7 ++++--- actor/helpers_test.go | 2 +- actor/mailbox.go | 2 +- actor/mailbox_test.go | 14 ++++++++------ actor/test_helpers.go | 4 ++-- actor/test_helpers_test.go | 2 +- go.mod | 2 +- 11 files changed, 35 insertions(+), 27 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 0385c81..a17acd0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -14,9 +14,8 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.55.2 + version: v1.56.0 - id: govulncheck uses: golang/govulncheck-action@v1 - # temporarily disabled - # - name: go mod tidy check - # uses: katexochen/go-tidy-check@v2 + - name: go mod tidy check + uses: katexochen/go-tidy-check@v2 diff --git a/Makefile b/Makefile index 4f9abaa..2cc555a 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ GO ?= go GOBIN ?= $$($(GO) env GOPATH)/bin GOLANGCI_LINT ?= $(GOBIN)/golangci-lint -GOLANGCI_LINT_VERSION ?= v1.55.2 +GOLANGCI_LINT_VERSION ?= v1.56.0 TEST_COVERAGE ?= $(GOBIN)/go-test-coverage .PHONY: install-golangcilint diff --git a/actor/actor_test.go b/actor/actor_test.go index cefd299..81a603b 100644 --- a/actor/actor_test.go +++ b/actor/actor_test.go @@ -26,7 +26,7 @@ func Test_NewWorker(t *testing.T) { w := NewWorker(workerFunc) assert.NotNil(t, w) - for i := 0; i < 10; i++ { + for range 10 { assert.Equal(t, WorkerContinue, w.DoWork(ctx)) assert.Equal(t, `🛠️`, <-workC) } @@ -57,7 +57,7 @@ func Test_Actor_Restart(t *testing.T) { w := newWorker() a := New(w) - for i := 0; i < 20; i++ { + for i := range 20 { a.Start() assertDoWork(t, w.doWorkC, i*workIterationsPerAssert) @@ -85,14 +85,14 @@ func Test_Actor_MultipleStartStop(t *testing.T) { ) // Calling Start() multiple times should have same effect as calling it once - for i := 0; i < count; i++ { + for range count { a.Start() } assertDoWork(t, w.doWorkC, 0) // Calling Stop() multiple times should have same effect as calling it once - for i := 0; i < count; i++ { + for range count { a.Stop() } @@ -139,11 +139,13 @@ func Test_Actor_OnStartOnStop(t *testing.T) { go a.OnStart() readySigC <- struct{}{} + assert.Equal(t, `🌞`, <-onStartC) assert.Empty(t, onStartC) go a.OnStop() readySigC <- struct{}{} + assert.Equal(t, `🌚`, <-onStopC) assert.Empty(t, onStopC) } @@ -161,6 +163,7 @@ func Test_Actor_OnStartOnStop(t *testing.T) { assert.Empty(t, onStartC) readySigC <- struct{}{} + assert.Equal(t, `🌞`, <-onStartC) assert.Empty(t, w.onStartC) assert.Empty(t, onStartC) @@ -172,6 +175,7 @@ func Test_Actor_OnStartOnStop(t *testing.T) { assert.Empty(t, onStopC) readySigC <- struct{}{} + assert.Equal(t, `🌚`, <-onStopC) assert.Empty(t, w.onStopC) assert.Empty(t, onStopC) @@ -207,6 +211,7 @@ func Test_Actor_StopAfterWorkerEnded(t *testing.T) { } p <- workIteration + workIteration++ return WorkerContinue @@ -341,6 +346,7 @@ func (w *worker) DoWork(c Context) WorkerStatus { } p <- w.workIteration + w.workIteration++ return WorkerContinue diff --git a/actor/combine_test.go b/actor/combine_test.go index 080d868..b5a00da 100644 --- a/actor/combine_test.go +++ b/actor/combine_test.go @@ -53,7 +53,7 @@ func Test_Combine_OptStopTogether(t *testing.T) { const actorsCount = 5 * 2 - for i := 0; i < actorsCount/2+1; i++ { + for i := range actorsCount/2 + 1 { onStartC := make(chan any, actorsCount) onStopC := make(chan any, actorsCount) onStart := OptOnStart(func(Context) { onStartC <- `🌞` }) @@ -103,7 +103,7 @@ func Test_Combine_OptOnStop_AfterActorStops(t *testing.T) { const actorsCount = 5 * 2 - for i := 0; i < actorsCount/2+1; i++ { + for i := range actorsCount/2 + 1 { onStopC, onStopOpt := createCombinedOnStopOption(t, 2) actors := createActors(actorsCount / 2) @@ -127,7 +127,7 @@ func Test_Combine_OptOnStop_AfterActorStops(t *testing.T) { func createActors(count int, opts ...Option) []Actor { actors := make([]Actor, count) - for i := 0; i < count; i++ { + for i := range count { actors[i] = createActor(i, opts...) } diff --git a/actor/context_test.go b/actor/context_test.go index 3d54bee..2dbcac8 100644 --- a/actor/context_test.go +++ b/actor/context_test.go @@ -36,7 +36,7 @@ func Test_Context_Stopping(t *testing.T) { func Test_Context_NewInstance(t *testing.T) { t.Parallel() - assert.NotSame(t, NewContext(), NewContext()) + assert.NotSame(t, NewContext(), NewContext()) //nolint:testifylint // relax } func Test_Context_ContextStarted(t *testing.T) { @@ -45,7 +45,7 @@ func Test_Context_ContextStarted(t *testing.T) { assertContextStarted(t, ContextStarted()) assertContextStringer(t, ContextStarted()) assertNoDeadline(t, ContextStarted()) - assert.Same(t, ContextStarted(), ContextStarted()) + assert.Same(t, ContextStarted(), ContextStarted()) //nolint:testifylint // relax } func Test_Context_ContextEnded(t *testing.T) { @@ -54,7 +54,7 @@ func Test_Context_ContextEnded(t *testing.T) { assertContextEnded(t, ContextEnded()) assertContextStringer(t, ContextEnded()) assertNoDeadline(t, ContextStarted()) - assert.Same(t, ContextEnded(), ContextEnded()) + assert.Same(t, ContextEnded(), ContextEnded()) //nolint:testifylint // relax } func Test_Context_Value(t *testing.T) { @@ -63,6 +63,7 @@ func Test_Context_Value(t *testing.T) { assertNoValue(t, func() Context { endedCtx := NewContext() endedCtx.End() + return endedCtx }()) assertNoValue(t, NewContext()) diff --git a/actor/helpers_test.go b/actor/helpers_test.go index 9c2ae26..3122f2b 100644 --- a/actor/helpers_test.go +++ b/actor/helpers_test.go @@ -7,7 +7,7 @@ import ( ) func drainC(c <-chan any, count int) { - for i := 0; i < count; i++ { + for range count { <-c } } diff --git a/actor/mailbox.go b/actor/mailbox.go index 00b6d03..5c0f047 100644 --- a/actor/mailbox.go +++ b/actor/mailbox.go @@ -52,7 +52,7 @@ func FanOut[T any, MS MailboxSender[T]](receiveC <-chan T, senders []MS) { // NewMailboxes returns slice of new Mailbox instances with specified count. func NewMailboxes[T any](count int, opt ...MailboxOption) []Mailbox[T] { mm := make([]Mailbox[T], count) - for i := 0; i < count; i++ { + for i := range count { mm[i] = NewMailbox[T](opt...) } diff --git a/actor/mailbox_test.go b/actor/mailbox_test.go index fbb5f67..ffe9f4d 100644 --- a/actor/mailbox_test.go +++ b/actor/mailbox_test.go @@ -46,11 +46,11 @@ func Test_Mailbox(t *testing.T) { // It is important to first send all data to Send() method to simulate excessive // incoming messages on this Mailbox. - for i := 0; i < sendMessagesCount; i++ { + for i := range sendMessagesCount { assert.NoError(t, m.Send(ContextStarted(), i)) } - for i := 0; i < sendMessagesCount; i++ { + for i := range sendMessagesCount { assert.Equal(t, <-m.ReceiveC(), i) } @@ -149,7 +149,7 @@ func Test_FanOut(t *testing.T) { // Produce data on inMbx go func() { - for i := 0; i < sendMessagesCount; i++ { + for i := range sendMessagesCount { assert.NoError(t, inMbx.Send(ContextStarted(), i)) } }() @@ -157,9 +157,10 @@ func Test_FanOut(t *testing.T) { // Assert that correct data is received by fanMbxx for _, m := range fanMbxx { go func(m Mailbox[any]) { - for i := 0; i < sendMessagesCount; i++ { + for i := range sendMessagesCount { assert.Equal(t, i, <-m.ReceiveC()) } + wg.Done() }(m) } @@ -234,7 +235,7 @@ func Test_Mailbox_OptEndAferReceivingAll(t *testing.T) { sendMessages := func(m Mailbox[any]) { t.Helper() - for i := 0; i < messagesCount; i++ { + for i := range messagesCount { assert.NoError(t, m.Send(ContextStarted(), `🥥`+tostr(i))) } } @@ -245,6 +246,7 @@ func Test_Mailbox_OptEndAferReceivingAll(t *testing.T) { for msg := range m.ReceiveC() { assert.Equal(t, `🥥`+tostr(gotMessages), msg) + gotMessages++ } @@ -328,7 +330,7 @@ func assertSendBlocking(t *testing.T, m Mailbox[any]) { go func() { err := m.Send(ctx, `🌹`) if err == nil { - assert.FailNow(t, "should not be able to send") + assert.FailNow(t, "should not be able to send") //nolint:testifylint // relax } close(testDoneSigC) diff --git a/actor/test_helpers.go b/actor/test_helpers.go index 86a142a..328150b 100644 --- a/actor/test_helpers.go +++ b/actor/test_helpers.go @@ -35,7 +35,7 @@ func AssertStartStopAtRandom(tb testing.TB, a Actor) { return } - for i := 0; i < 1000; i++ { + for range 1000 { if randInt32(tb)%2 == 0 { a.Start() } else { @@ -94,7 +94,7 @@ func randInt32WithReader(tb testing.TB, randReader io.Reader) int32 { } result := int32(0) - for i := 0; i < 4; i++ { + for i := range 4 { result <<= 8 result += int32(b[i]) } diff --git a/actor/test_helpers_test.go b/actor/test_helpers_test.go index a9ce61c..eb754e4 100644 --- a/actor/test_helpers_test.go +++ b/actor/test_helpers_test.go @@ -63,7 +63,7 @@ func Test_RandInt32WithReader(t *testing.T) { t.Parallel() v := make(map[int32]struct{}) - for i := 0; i < 10000; i++ { + for range 10000 { v[RandInt32WithReader(t, rand.Reader)] = struct{}{} } diff --git a/go.mod b/go.mod index b819d87..bdfe1a1 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/vladopajic/go-actor -go 1.21 +go 1.22 require ( github.com/gammazero/deque v0.2.1 From 7d0be18fc9fdebb1d09652f205ccc13ae41f4ace Mon Sep 17 00:00:00 2001 From: vladopajic Date: Fri, 9 Feb 2024 11:04:10 +0100 Subject: [PATCH 3/7] simplify mailbox capacity options (#72) --- .golangci.yml | 2 +- actor/actor.go | 4 ++-- actor/context.go | 3 ++- actor/export_test.go | 4 ++-- actor/mailbox.go | 15 ++++++++++----- actor/options.go | 25 ++++--------------------- actor/options_test.go | 28 ---------------------------- actor/queue.go | 5 ++--- actor/queue_test.go | 16 +++++----------- 9 files changed, 28 insertions(+), 74 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index fc256c0..59ee0b1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -157,7 +157,7 @@ linters-settings: disable: - fieldalignment lll: - line-length: 100 + line-length: 90 tab-width: 1 nlreturn: block-size: 5 diff --git a/actor/actor.go b/actor/actor.go index 4b72fed..c01ca6a 100644 --- a/actor/actor.go +++ b/actor/actor.go @@ -50,8 +50,8 @@ type StartableWorker interface { // initialize Worker as it will be called only once. // // Context is provided in case when Actor is stopped early and OnStop should terminated - // with initialization. This is same Context as one which will be provided to DoWork method - // in later stages of Worker lifecycle. + // with initialization. This is same Context as one which will be provided to DoWork + // method in later stages of Worker lifecycle. OnStart(ctx Context) } diff --git a/actor/context.go b/actor/context.go index aac8664..7f5e30a 100644 --- a/actor/context.go +++ b/actor/context.go @@ -78,7 +78,8 @@ func (c *context) Err() error { return err } -func (*context) Value(key any) any { //nolint:revive // needed to implement context.Context +//nolint:revive // needed to implement context.Context +func (*context) Value(key any) any { return nil } diff --git a/actor/export_test.go b/actor/export_test.go index f2cfa34..20f082a 100644 --- a/actor/export_test.go +++ b/actor/export_test.go @@ -55,8 +55,8 @@ func (w *mailboxWorker[T]) Queue() *queue[T] { return w.queue } -func NewQueue[T any](capacity, minimum int) *queue[T] { - return newQueue[T](capacity, minimum) +func NewQueue[T any](capacity int) *queue[T] { + return newQueue[T](capacity) } func (q *queue[T]) Cap() int { diff --git a/actor/mailbox.go b/actor/mailbox.go index 5c0f047..f0eae4d 100644 --- a/actor/mailbox.go +++ b/actor/mailbox.go @@ -59,12 +59,17 @@ func NewMailboxes[T any](count int, opt ...MailboxOption) []Mailbox[T] { return mm } -const mbxChanBufferCap = 64 +const ( + mbxChanBufferCap = 64 + minQueueCapacity = mbxChanBufferCap +) // NewMailbox returns new local Mailbox implementation. -// Mailbox is much like native go channel, except that writing to the Mailbox -// will never block, all messages are going to be queued and Actors on -// receiving end of the Mailbox will get all messages in FIFO order. +// +// Default Mailbox closely resembles a native Go channel, with the key distinction that +// writing to the Mailbox will never cause blocking, and all messages are queued without +// limitations. Mailbox can also behave exactly the same as native Go channel when option +// `OptAsChan` is used. func NewMailbox[T any](opt ...MailboxOption) Mailbox[T] { options := newOptions(opt).Mailbox @@ -190,7 +195,7 @@ func newMailboxWorker[T any]( receiveC chan T, options optionsMailbox, ) *mailboxWorker[T] { - queue := newQueue[T](options.Capacity, options.MinCapacity) + queue := newQueue[T](options.Capacity) return &mailboxWorker[T]{ sendC: sendC, diff --git a/actor/options.go b/actor/options.go index a8a3e0b..cab870e 100644 --- a/actor/options.go +++ b/actor/options.go @@ -22,32 +22,16 @@ func OptOnStop(f func()) Option { } } -// OptCapacity sets initial Mailbox queue capacity. -// Value must be power of 2. +// OptCapacity sets Mailbox queue capacity. +// When `OptAsChan` is used together with `OptCapacity` capacity value +// will set be set to underlaying channel. func OptCapacity(capacity int) MailboxOption { return func(o *options) { o.Mailbox.Capacity = capacity } } -// OptMinCapacity sets minimum Mailbox queue capacity. -// Value must be power of 2. -func OptMinCapacity(minCapacity int) MailboxOption { - return func(o *options) { - o.Mailbox.MinCapacity = minCapacity - } -} - -// OptMailbox sets all Mailbox capacity options at once. -func OptMailbox(capacity, minCapacity int) MailboxOption { - return func(o *options) { - o.Mailbox.Capacity = capacity - o.Mailbox.MinCapacity = minCapacity - } -} - -// OptAsChan makes Mailbox to function as wrapper for -// native go channel. +// OptAsChan transforms the Mailbox into a wrapper for the native Go channel. func OptAsChan() MailboxOption { return func(o *options) { o.Mailbox.AsChan = true @@ -112,7 +96,6 @@ type optionsCombined struct { type optionsMailbox struct { AsChan bool Capacity int - MinCapacity int StopAfterReceivingAll bool } diff --git a/actor/options_test.go b/actor/options_test.go index 44903af..e93bebb 100644 --- a/actor/options_test.go +++ b/actor/options_test.go @@ -56,34 +56,6 @@ func testMailboxOptions(t *testing.T) { { // Assert that OptCapacity will be set opts := NewOptions(OptCapacity(16)) assert.Equal(t, 16, opts.Mailbox.Capacity) - assert.Equal(t, 0, opts.Mailbox.MinCapacity) - - assert.Empty(t, opts.Actor) - assert.Empty(t, opts.Combined) - } - - { // Assert that OptMinCapacity will be set - opts := NewOptions(OptMinCapacity(32)) - assert.Equal(t, 0, opts.Mailbox.Capacity) - assert.Equal(t, 32, opts.Mailbox.MinCapacity) - - assert.Empty(t, opts.Actor) - assert.Empty(t, opts.Combined) - } - - { // Assert that OptCapacity and OptMinCapacity will be set - opts := NewOptions(OptCapacity(16), OptMinCapacity(32)) - assert.Equal(t, 16, opts.Mailbox.Capacity) - assert.Equal(t, 32, opts.Mailbox.MinCapacity) - - assert.Empty(t, opts.Actor) - assert.Empty(t, opts.Combined) - } - - { // Assert that OptCapacity and OptMinCapacity will be set - opts := NewOptions(OptMailbox(16, 32)) - assert.Equal(t, 16, opts.Mailbox.Capacity) - assert.Equal(t, 32, opts.Mailbox.MinCapacity) assert.Empty(t, opts.Actor) assert.Empty(t, opts.Combined) diff --git a/actor/queue.go b/actor/queue.go index 2df5c9c..9589974 100644 --- a/actor/queue.go +++ b/actor/queue.go @@ -4,9 +4,8 @@ import ( queueImpl "github.com/gammazero/deque" ) -const minQueueCapacity = 64 - -func newQueue[T any](capacity, minimum int) *queue[T] { +func newQueue[T any](capacity int) *queue[T] { + minimum := capacity if minimum < minQueueCapacity { minimum = minQueueCapacity } diff --git a/actor/queue_test.go b/actor/queue_test.go index 7779a70..40c9115 100644 --- a/actor/queue_test.go +++ b/actor/queue_test.go @@ -11,7 +11,7 @@ import ( func TestQueue_Basic(t *testing.T) { t.Parallel() - q := NewQueue[int](0, 0) + q := NewQueue[int](0) assert.Equal(t, 0, q.Cap()) assert.Equal(t, 0, q.Len()) @@ -53,8 +53,8 @@ func TestQueue_Cap(t *testing.T) { t.Parallel() { - q := NewQueue[any](0, 10) - assert.Equal(t, 0, q.Cap()) + q := NewQueue[any](10) + assert.Equal(t, MinQueueCapacity, q.Cap()) assert.Equal(t, 0, q.Len()) q.PushBack(`🌊`) @@ -64,21 +64,15 @@ func TestQueue_Cap(t *testing.T) { } { - q := NewQueue[any](10, 10) + q := NewQueue[any](10) assert.Equal(t, MinQueueCapacity, q.Cap()) assert.Equal(t, 0, q.Len()) } { - q := NewQueue[int](MinQueueCapacity*2, 10) + q := NewQueue[any](MinQueueCapacity * 2) assert.Equal(t, MinQueueCapacity*2, q.Cap()) assert.Equal(t, 0, q.Len()) - } - - { - q := NewQueue[any](0, MinQueueCapacity*2) - assert.Equal(t, 0, q.Cap()) - assert.Equal(t, 0, q.Len()) q.PushBack(`🌊`) From a148b0a4fd63641f6cf3e2fddaa44e466d95b86a Mon Sep 17 00:00:00 2001 From: Vlado Date: Fri, 9 Feb 2024 11:09:05 +0100 Subject: [PATCH 4/7] add markdown code block language --- docs/best_practices.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/best_practices.md b/docs/best_practices.md index ac4461a..956d7d0 100644 --- a/docs/best_practices.md +++ b/docs/best_practices.md @@ -6,7 +6,7 @@ To enhance the code quality of projects that heavily rely on the actor model and Projects that fully relay on actor model and `go-actor` library shouldn't use any synchronization primitives from `sync` package. Therefore repositories based on `go-actor` could add linter that will warn them if `sync` package is used, eg: -``` +```yml linters-settings: forbidigo: forbid: @@ -23,7 +23,7 @@ While the general rule is to avoid `sync` package usage in actor-based code, the Workers should always respond to `Context.Done()` channel and return `actor.WorkerEnd` status in order to end it's actor. As a rule of thumb it's advised to always list this case first since it should be included in every `select` statement. -``` +```go func (w *worker) DoWork(ctx actor.Context) actor.WorkerStatus { select { case <-ctx.Done(): From b7b48d31525c098e17746355dd59f47c3b2a16ae Mon Sep 17 00:00:00 2001 From: Vlado Date: Fri, 9 Feb 2024 13:38:25 +0100 Subject: [PATCH 5/7] docs improvement --- docs/best_practices.md | 62 +++++++++++++++++++++++++++++++++++++----- docs/common_hurdles.md | 30 +++++++++++++++++++- 2 files changed, 84 insertions(+), 8 deletions(-) diff --git a/docs/best_practices.md b/docs/best_practices.md index 956d7d0..0f51b1f 100644 --- a/docs/best_practices.md +++ b/docs/best_practices.md @@ -24,18 +24,66 @@ While the general rule is to avoid `sync` package usage in actor-based code, the Workers should always respond to `Context.Done()` channel and return `actor.WorkerEnd` status in order to end it's actor. As a rule of thumb it's advised to always list this case first since it should be included in every `select` statement. ```go -func (w *worker) DoWork(ctx actor.Context) actor.WorkerStatus { +func (w *fooWorker) DoWork(ctx actor.Context) actor.WorkerStatus { + select { + case <-ctx.Done(): // <----------------------- handle ctx.Done() first + return actor.WorkerEnd + + case msg := <-w.mbx.ReceiveC(): + handleFoo(msg) + } +} +``` + +## Check channel closed indicator in `DoWork` + +Every case statement in `DoWork` should handle case when channel is closed. In these cases worker should end execution; or it can perform any other logic that is necessery. + +```go +func (w *fooWorker) DoWork(ctx actor.Context) actor.WorkerStatus { select { case <-ctx.Done(): return actor.WorkerEnd - case <-w.fooMbx.ReceiveC(): - handleFoo() - case <-w.barMbx.ReceiveC(): - handleBar() + + case msg, ok := <-w.mbx.ReceiveC(): + if !ok { // <----------------------- handle channel close (mailbox stop) case + return actor.WorkerEnd + } + + handleFoo(msg) } } ``` - + +## Combine multiple actors to singe actor + +`actor.Combine(...)` is vary handy to combine multiple actors to single actor. + +```go +type fooActor struct { + actor.Actor + mbx actor.Mailbox[any] + ... +} + +func NewFooActor() *fooActor { + mbx := actor.NewMailbox[any]() + + a1 := actor.New(&fooWorker{mbx: mbx}) + a2 := actor.New(&fooWorker{mbx: mbx}) + + return &fooActor{ + mbx: mbx, + Actor: actor.Combine(mbx, a1, a2).Build() // <------- combine all actors to single actor and initialize embeded actor of fooActor struct. + } // when calling fooActor.Start() it will start all actors at once. +} + +func (f *fooActor) OnMessage(ctx context.Context, msg any) error { + return f.mbx.Send(ctx, msg) +} + +``` + --- -`// page is not yet complete` \ No newline at end of file +This page is not yet complete. \ No newline at end of file diff --git a/docs/common_hurdles.md b/docs/common_hurdles.md index b818acd..4ec9d87 100644 --- a/docs/common_hurdles.md +++ b/docs/common_hurdles.md @@ -8,6 +8,34 @@ One of the most common hurdles is the case where actors are not started. This is Never forget that `actor.Mailbox` is also an actor, and it needs to be started. +**Embeded Actor interface is overriden** + +When embeding `actor.Actor` interface make sure not to override methods of this interface in structre that has embeded it. Otherwise make sure to call embeded actor's Start() and Stop() methods. + +```go +type fooActor struct { + actor.Actor + ... +} + +func NewFooActor() *fooActor { + return &fooActor{ + Actor: actor.New(&fooWorker{...}) + ... + } +} + +func(f *fooActor) Start() { // <--- warning: calling fooActor.Start() will override fooActor.Actor.Start() method. + ... // therfore calling this method will not execute worker that was itended +} // to be excuted with fooActor. + // if this method is necessery then make sure to call `f.Actor.Start()` manually here. + +func(f *fooActor) Stop() { // <---- similar problem as described above. + ... +} + +``` + ## Default case is undesirable Workers should always block when there isn't anything to work on; therefore, their `select` statements shouldn't have a `default` case. If workers do not block, they will simply waste computation cycles. @@ -32,4 +60,4 @@ func (w *consumeWorker) DoWork(ctx actor.Context) actor.WorkerStatus { --- -// Your contribution is valuable; if you have encountered any challenges, please share your experiences. \ No newline at end of file +Your contribution is valuable; if you have encountered any challenges, please share your experiences. \ No newline at end of file From 2aa18e7547436210070f85ec2f62d92591e4e360 Mon Sep 17 00:00:00 2001 From: vladopajic Date: Fri, 9 Feb 2024 13:40:38 +0100 Subject: [PATCH 6/7] Update common_hurdles.md --- docs/common_hurdles.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/common_hurdles.md b/docs/common_hurdles.md index 4ec9d87..3f844c5 100644 --- a/docs/common_hurdles.md +++ b/docs/common_hurdles.md @@ -25,12 +25,12 @@ func NewFooActor() *fooActor { } } -func(f *fooActor) Start() { // <--- warning: calling fooActor.Start() will override fooActor.Actor.Start() method. - ... // therfore calling this method will not execute worker that was itended -} // to be excuted with fooActor. - // if this method is necessery then make sure to call `f.Actor.Start()` manually here. +func(f *fooActor) Start() { // warning: calling fooActor.Start() will override fooActor.Actor.Start() method. + ... // therfore calling this method will not execute worker that was itended +} // to be excuted with fooActor. + // if this method is necessery then make sure to call `f.Actor.Start()` manually here. -func(f *fooActor) Stop() { // <---- similar problem as described above. +func(f *fooActor) Stop() { // similar problem as described above. ... } @@ -60,4 +60,4 @@ func (w *consumeWorker) DoWork(ctx actor.Context) actor.WorkerStatus { --- -Your contribution is valuable; if you have encountered any challenges, please share your experiences. \ No newline at end of file +Your contribution is valuable; if you have encountered any challenges, please share your experiences. From b18ff4ae8c6392e8501a496f0611ad1e568a94a6 Mon Sep 17 00:00:00 2001 From: vladopajic Date: Fri, 9 Feb 2024 13:41:26 +0100 Subject: [PATCH 7/7] Update best_practices.md --- docs/best_practices.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/best_practices.md b/docs/best_practices.md index 0f51b1f..0a9b8f3 100644 --- a/docs/best_practices.md +++ b/docs/best_practices.md @@ -74,8 +74,8 @@ func NewFooActor() *fooActor { return &fooActor{ mbx: mbx, - Actor: actor.Combine(mbx, a1, a2).Build() // <------- combine all actors to single actor and initialize embeded actor of fooActor struct. - } // when calling fooActor.Start() it will start all actors at once. + Actor: actor.Combine(mbx, a1, a2).Build() // combine all actors to single actor and initialize embeded actor of fooActor struct. + } // when calling fooActor.Start() it will start all actors at once. } func (f *fooActor) OnMessage(ctx context.Context, msg any) error { @@ -86,4 +86,4 @@ func (f *fooActor) OnMessage(ctx context.Context, msg any) error { --- -This page is not yet complete. \ No newline at end of file +This page is not yet complete.