From 38cf1bfe4f974a8334dd01b289f49522db0f6709 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Sat, 22 Feb 2025 16:35:19 +0900 Subject: [PATCH 01/37] Implement debouncing throttler --- go.mod | 5 ++- go.sum | 2 + pkg/throttle/throttler.go | 86 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 pkg/throttle/throttler.go diff --git a/go.mod b/go.mod index 3f99000a4..496bde4d6 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,10 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -require github.com/pierrec/lz4/v4 v4.1.15 // indirect +require ( + github.com/pierrec/lz4/v4 v4.1.15 // indirect + golang.org/x/time v0.10.0 // indirect +) require ( github.com/beorn7/perks v1.0.1 // indirect diff --git a/go.sum b/go.sum index 3bd98928d..14665358d 100644 --- a/go.sum +++ b/go.sum @@ -575,6 +575,8 @@ golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= +golang.org/x/time v0.10.0 h1:3usCWA8tQn0L8+hFJQNgzpWbd89begxN66o1Ojdn5L4= +golang.org/x/time v0.10.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= diff --git a/pkg/throttle/throttler.go b/pkg/throttle/throttler.go new file mode 100644 index 000000000..5101bb6bf --- /dev/null +++ b/pkg/throttle/throttler.go @@ -0,0 +1,86 @@ +/* + * Copyright 2025 The Yorkie Authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package throttle provide event timing control components +package throttle + +import ( + "context" + "fmt" + "sync/atomic" + "time" + + "golang.org/x/time/rate" +) + +// Throttler provides a combined throttling and debouncing mechanism +// that ensures eventual consistency. It calls the callback immediately +// if allowed by the rate limiter; otherwise, it schedules a trailing callback. +type Throttler struct { + lim *rate.Limiter + pending int32 // 0 means false, 1 means true +} + +// New creates a new instance with the specified throttle intervals. +func New(window time.Duration) *Throttler { + dt := &Throttler{ + lim: rate.NewLimiter(rate.Every(window), 1), + pending: 0, + } + return dt +} + +// Execute attempts to run the provided callback function immediately if the rate limiter allows it. +// If the rate limiter does not allow immediate execution, this function blocks until the next token +// is available and then runs the callback. If there is already a pending callback, Execute returns +// immediately. This mechanism ensures that the final callback is executed after the final event, +// providing eventual consistency. +func (t *Throttler) Execute(ctx context.Context, callback func() error) error { + if t.lim.Allow() { + return callback() + } + + if !atomic.CompareAndSwapInt32(&t.pending, 0, 1) { + return nil + } + + if err := t.lim.Wait(ctx); err != nil { + return fmt.Errorf("wait for limiter: %w", err) + } + + atomic.StoreInt32(&t.pending, 0) + return callback() +} + +// ExecuteOrSchedule is the asynchronous counterpart to Execute. It runs the provided callback +// immediately if the rate limiter allows it. Otherwise, it schedules the callback to run +// once the next token becomes available and returns immediately. If there is already a pending +// callback, this function returns without scheduling another one. +func (t *Throttler) ExecuteOrSchedule(callback func()) { + if t.lim.Allow() { + callback() + return + } + + if !atomic.CompareAndSwapInt32(&t.pending, 0, 1) { + return + } + delay := t.lim.Reserve().Delay() + time.AfterFunc(delay, func() { + atomic.StoreInt32(&t.pending, 0) + callback() + }) +} From 738617064c33ed9515c126e1db2f608d4823e08d Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Sat, 22 Feb 2025 17:51:52 +0900 Subject: [PATCH 02/37] Add tests --- pkg/throttle/throttler_test.go | 290 +++++++++++++++++++++++++++++++++ 1 file changed, 290 insertions(+) create mode 100644 pkg/throttle/throttler_test.go diff --git a/pkg/throttle/throttler_test.go b/pkg/throttle/throttler_test.go new file mode 100644 index 000000000..864f1c85d --- /dev/null +++ b/pkg/throttle/throttler_test.go @@ -0,0 +1,290 @@ +/* + * Copyright 2025 The Yorkie Authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package throttle provide event timing control components +package throttle + +import ( + "context" + "errors" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +// TestSynchronousExecute verifies that synchronous calls to the throttler +// execute the callback immediately without any delays. +func TestSynchronousExecute(t *testing.T) { + ctx := context.Background() + throttleWindow := 10 * time.Millisecond + + t.Run("Single call executes callback exactly once", func(t *testing.T) { + th := New(throttleWindow) + var callCount int32 + callback := func() error { + atomic.AddInt32(&callCount, 1) + return nil + } + + err := th.Execute(ctx, callback) + assert.NoError(t, err) + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + }) + + t.Run("Ten consecutive calls execute callback each time", func(t *testing.T) { + th := New(throttleWindow) + var callCount int32 + callback := func() error { + atomic.AddInt32(&callCount, 1) + return nil + } + + for i := 0; i < 10; i++ { + assert.NoError(t, th.Execute(ctx, callback)) + } + assert.Equal(t, int32(10), atomic.LoadInt32(&callCount)) + }) +} + +// TestConcurrentExecute verifies that the throttler behaves as expected under concurrent invocations. +func TestConcurrentExecute(t *testing.T) { + ctx := context.Background() + throttleWindow := 10 * time.Millisecond + + // In this test, many concurrent calls are made. The throttler should execute one immediate call + // and then schedule a trailing call, resulting in exactly two executions. + t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { + th := New(throttleWindow) + var callCount int32 + callback := func() error { + atomic.AddInt32(&callCount, 1) + return nil + } + + for i := 0; i < 1000; i++ { + go func() { + assert.NoError(t, th.Execute(ctx, callback)) + }() + } + // Allow enough time for the trailing call to be scheduled. + time.Sleep(throttleWindow) + // Expect exactly one immediate and one trailing callback execution. + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + }) + + // This test simulates a continuous stream of events. + // It triggers multiple concurrent calls at a regular interval and checks that throttling + // limits the total number of callback invocations to one per window plus one trailing call. + t.Run("Throttling over continuous event stream", func(t *testing.T) { + th := New(throttleWindow) + numWindows := int32(10) + totalDuration := throttleWindow * time.Duration(numWindows) + eventsPerWindow := 100 + interval := throttleWindow / time.Duration(eventsPerWindow) + ticker := time.NewTicker(interval) + defer ticker.Stop() + + timeCtx, cancel := context.WithTimeout(ctx, totalDuration) + defer cancel() + + var callCount int32 + callback := func() error { + atomic.AddInt32(&callCount, 1) + return nil + } + + // Continuously trigger events until the timeout. + for { + select { + case <-ticker.C: + // Each tick triggers multiple concurrent calls. + for i := 0; i < 10; i++ { + go func() { + assert.NoError(t, th.Execute(ctx, callback)) + }() + } + case <-timeCtx.Done(): + // Allow any trailing call to execute. + time.Sleep(throttleWindow) + // Expect one execution per window plus one trailing call. + assert.Equal(t, numWindows+1, atomic.LoadInt32(&callCount)) + return + } + } + }) +} + +// TestCallbackErrorPropagation checks that errors returned by the callback +// are immediately propagated back to the caller. +func TestCallbackErrorPropagation(t *testing.T) { + ctx := context.Background() + throttleWindow := 10 * time.Millisecond + expectedErr := errors.New("callback error") + + t.Run("Immediate callback error is propagated", func(t *testing.T) { + th := New(throttleWindow) + callback := func() error { + return expectedErr + } + err := th.Execute(ctx, callback) + assert.ErrorIs(t, err, expectedErr) + }) +} + +// TestContextCancellation verifies the throttler's behavior when the context +// expires (deadline exceeded) or is canceled. +func TestContextCancellation(t *testing.T) { + throttleWindow := 10 * time.Millisecond + + // In this test the context deadline is shorter than the throttle window. + // The trailing call should fail with a deadline exceeded error. + t.Run("Trailing call fails due to context deadline exceeded", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), throttleWindow/2) + defer cancel() + th := New(throttleWindow) + var callCount int32 + callback := func() error { + atomic.AddInt32(&callCount, 1) + return nil + } + + // The first call executes immediately. + assert.NoError(t, th.Execute(ctx, callback)) + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + // The second call is delayed and should eventually time out. + err := th.Execute(ctx, callback) + assert.ErrorAs(t, err, &context.DeadlineExceeded) + // Ensure the callback was not executed a second time. + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + }) + + // This test verifies that when the context is canceled, + // any pending (trailing) call does not execute. + t.Run("Trailing call fails due to context cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + th := New(throttleWindow) + var callCount int32 + callback := func() error { + atomic.AddInt32(&callCount, 1) + return nil + } + + // First call executes immediately. + assert.NoError(t, th.Execute(ctx, callback)) + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + // Launch a trailing call that will be affected by cancellation. + go func() { + err := th.Execute(ctx, callback) + assert.ErrorAs(t, err, &context.Canceled) + }() + // Cancel the context to cancel any pending trailing call. + cancel() + // Verify that the trailing call was not executed. + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + }) +} + +// TestCallAsyncDelayed verifies that if the rate limiter does not allow an immediate call, +// ExecuteOrSchedule schedules a trailing callback which eventually executes. +func TestExecuteOrSchedule(t *testing.T) { + throttleWindow := 10 * time.Millisecond + th := New(throttleWindow) + var callCount int32 + done := make(chan struct{}, 2) + defer close(done) + + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + // First call should be allowed immediately. + th.ExecuteOrSchedule(callback) + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + + // Second call should be rate limited and schedule a trailing call. + before := time.Now() + // it schedules only one callback + for range 100 { + th.ExecuteOrSchedule(callback) + } + duration := time.Since(before) + assert.Less(t, duration, throttleWindow/10, "Execution time exceeded expected threshold") + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + time.Sleep(throttleWindow * 2) + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) +} + +// TestConcurrentExecuteOrSchedule verifies that the throttler behaves as expected under concurrent invocations. +func TestConcurrentExecuteOrSchedule(t *testing.T) { + ctx := context.Background() + throttleWindow := 10 * time.Millisecond + + t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { + th := New(throttleWindow) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + for i := 0; i < 100; i++ { + th.ExecuteOrSchedule(callback) + } + // Allow enough time for the trailing call to be scheduled. + time.Sleep(throttleWindow * 2) + // Expect exactly one immediate and one trailing callback execution. + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + }) + + // This test simulates a continuous stream of events. + // It triggers multiple concurrent calls at a regular interval and checks that throttling + // limits the total number of callback invocations to one per window plus one trailing call. + t.Run("Throttling over continuous event stream", func(t *testing.T) { + th := New(throttleWindow) + numWindows := int32(10) + totalDuration := throttleWindow * time.Duration(numWindows) + eventsPerWindow := 100 + interval := throttleWindow / time.Duration(eventsPerWindow) + ticker := time.NewTicker(interval) + defer ticker.Stop() + + timeCtx, cancel := context.WithTimeout(ctx, totalDuration) + defer cancel() + + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + // Continuously trigger events until the timeout. + for { + select { + case <-ticker.C: + // Each tick triggers multiple concurrent calls. + for i := 0; i < 10; i++ { + th.ExecuteOrSchedule(callback) + } + case <-timeCtx.Done(): + // Allow any trailing call to execute. + time.Sleep(throttleWindow * 2) + // Expect one execution per window plus one trailing call. + assert.Equal(t, numWindows+1, atomic.LoadInt32(&callCount)) + return + } + } + }) +} From 87a50e960bbb93247afd67ec2368a5f21d26cb96 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Mon, 24 Feb 2025 11:41:16 +0900 Subject: [PATCH 03/37] Rename package `throttle` -> `limit` --- pkg/{throttle => limit}/throttler.go | 0 pkg/{throttle => limit}/throttler_test.go | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename pkg/{throttle => limit}/throttler.go (100%) rename pkg/{throttle => limit}/throttler_test.go (100%) diff --git a/pkg/throttle/throttler.go b/pkg/limit/throttler.go similarity index 100% rename from pkg/throttle/throttler.go rename to pkg/limit/throttler.go diff --git a/pkg/throttle/throttler_test.go b/pkg/limit/throttler_test.go similarity index 100% rename from pkg/throttle/throttler_test.go rename to pkg/limit/throttler_test.go From ca8485eb0a9a2850df60630c17e3da046db4556b Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Mon, 24 Feb 2025 11:42:27 +0900 Subject: [PATCH 04/37] Solve concurrent error in test - wait goroutine done in test --- pkg/limit/throttler_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/limit/throttler_test.go b/pkg/limit/throttler_test.go index 864f1c85d..2abb4d610 100644 --- a/pkg/limit/throttler_test.go +++ b/pkg/limit/throttler_test.go @@ -188,14 +188,17 @@ func TestContextCancellation(t *testing.T) { assert.NoError(t, th.Execute(ctx, callback)) assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) // Launch a trailing call that will be affected by cancellation. + done := make(chan struct{}) go func() { + defer close(done) err := th.Execute(ctx, callback) assert.ErrorAs(t, err, &context.Canceled) + // Verify that the trailing call was not executed. + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) }() // Cancel the context to cancel any pending trailing call. cancel() - // Verify that the trailing call was not executed. - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + <-done }) } From f7b9d6d9fc268a41176e992ce3705b2b7d2f95c0 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Mon, 24 Feb 2025 12:58:20 +0900 Subject: [PATCH 05/37] Refactor test - use const - use wait group --- pkg/limit/throttler_test.go | 86 +++++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/pkg/limit/throttler_test.go b/pkg/limit/throttler_test.go index 2abb4d610..8eddb2a48 100644 --- a/pkg/limit/throttler_test.go +++ b/pkg/limit/throttler_test.go @@ -20,6 +20,7 @@ package throttle import ( "context" "errors" + "sync" "sync/atomic" "testing" "time" @@ -31,7 +32,7 @@ import ( // execute the callback immediately without any delays. func TestSynchronousExecute(t *testing.T) { ctx := context.Background() - throttleWindow := 10 * time.Millisecond + const throttleWindow = 10 * time.Millisecond t.Run("Single call executes callback exactly once", func(t *testing.T) { th := New(throttleWindow) @@ -54,7 +55,7 @@ func TestSynchronousExecute(t *testing.T) { return nil } - for i := 0; i < 10; i++ { + for range 10 { assert.NoError(t, th.Execute(ctx, callback)) } assert.Equal(t, int32(10), atomic.LoadInt32(&callCount)) @@ -64,25 +65,30 @@ func TestSynchronousExecute(t *testing.T) { // TestConcurrentExecute verifies that the throttler behaves as expected under concurrent invocations. func TestConcurrentExecute(t *testing.T) { ctx := context.Background() - throttleWindow := 10 * time.Millisecond + const throttleWindow = 100 * time.Millisecond // In this test, many concurrent calls are made. The throttler should execute one immediate call // and then schedule a trailing call, resulting in exactly two executions. t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { th := New(throttleWindow) + const numRoutines = 1000 var callCount int32 callback := func() error { atomic.AddInt32(&callCount, 1) return nil } - for i := 0; i < 1000; i++ { + var wg sync.WaitGroup + wg.Add(numRoutines) + + for range numRoutines { go func() { assert.NoError(t, th.Execute(ctx, callback)) + wg.Done() }() } - // Allow enough time for the trailing call to be scheduled. - time.Sleep(throttleWindow) + + wg.Wait() // Expect exactly one immediate and one trailing callback execution. assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) }) @@ -91,11 +97,16 @@ func TestConcurrentExecute(t *testing.T) { // It triggers multiple concurrent calls at a regular interval and checks that throttling // limits the total number of callback invocations to one per window plus one trailing call. t.Run("Throttling over continuous event stream", func(t *testing.T) { - th := New(throttleWindow) - numWindows := int32(10) + const ( + numWindows = 10 + eventPerWindow = 100 + numRoutines = 10 + ) totalDuration := throttleWindow * time.Duration(numWindows) - eventsPerWindow := 100 - interval := throttleWindow / time.Duration(eventsPerWindow) + interval := throttleWindow / time.Duration(eventPerWindow) + + th := New(throttleWindow) + ticker := time.NewTicker(interval) defer ticker.Stop() @@ -113,7 +124,7 @@ func TestConcurrentExecute(t *testing.T) { select { case <-ticker.C: // Each tick triggers multiple concurrent calls. - for i := 0; i < 10; i++ { + for range numRoutines { go func() { assert.NoError(t, th.Execute(ctx, callback)) }() @@ -122,7 +133,7 @@ func TestConcurrentExecute(t *testing.T) { // Allow any trailing call to execute. time.Sleep(throttleWindow) // Expect one execution per window plus one trailing call. - assert.Equal(t, numWindows+1, atomic.LoadInt32(&callCount)) + assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) return } } @@ -133,7 +144,7 @@ func TestConcurrentExecute(t *testing.T) { // are immediately propagated back to the caller. func TestCallbackErrorPropagation(t *testing.T) { ctx := context.Background() - throttleWindow := 10 * time.Millisecond + const throttleWindow = 10 * time.Millisecond expectedErr := errors.New("callback error") t.Run("Immediate callback error is propagated", func(t *testing.T) { @@ -149,7 +160,7 @@ func TestCallbackErrorPropagation(t *testing.T) { // TestContextCancellation verifies the throttler's behavior when the context // expires (deadline exceeded) or is canceled. func TestContextCancellation(t *testing.T) { - throttleWindow := 10 * time.Millisecond + const throttleWindow = 10 * time.Millisecond // In this test the context deadline is shorter than the throttle window. // The trailing call should fail with a deadline exceeded error. @@ -188,29 +199,32 @@ func TestContextCancellation(t *testing.T) { assert.NoError(t, th.Execute(ctx, callback)) assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) // Launch a trailing call that will be affected by cancellation. - done := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { - defer close(done) err := th.Execute(ctx, callback) assert.ErrorAs(t, err, &context.Canceled) // Verify that the trailing call was not executed. assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + wg.Done() }() // Cancel the context to cancel any pending trailing call. cancel() - <-done + wg.Wait() }) } // TestCallAsyncDelayed verifies that if the rate limiter does not allow an immediate call, // ExecuteOrSchedule schedules a trailing callback which eventually executes. func TestExecuteOrSchedule(t *testing.T) { - throttleWindow := 10 * time.Millisecond + const ( + throttleWindow = 10 * time.Millisecond + numRoutines = 100 + ) th := New(throttleWindow) - var callCount int32 - done := make(chan struct{}, 2) - defer close(done) + var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) } @@ -222,7 +236,7 @@ func TestExecuteOrSchedule(t *testing.T) { // Second call should be rate limited and schedule a trailing call. before := time.Now() // it schedules only one callback - for range 100 { + for range numRoutines { th.ExecuteOrSchedule(callback) } duration := time.Since(before) @@ -234,17 +248,18 @@ func TestExecuteOrSchedule(t *testing.T) { // TestConcurrentExecuteOrSchedule verifies that the throttler behaves as expected under concurrent invocations. func TestConcurrentExecuteOrSchedule(t *testing.T) { - ctx := context.Background() - throttleWindow := 10 * time.Millisecond + const throttleWindow = 10 * time.Millisecond t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { th := New(throttleWindow) + const numRoutines = 100 + var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) } - for i := 0; i < 100; i++ { + for range numRoutines { th.ExecuteOrSchedule(callback) } // Allow enough time for the trailing call to be scheduled. @@ -257,15 +272,20 @@ func TestConcurrentExecuteOrSchedule(t *testing.T) { // It triggers multiple concurrent calls at a regular interval and checks that throttling // limits the total number of callback invocations to one per window plus one trailing call. t.Run("Throttling over continuous event stream", func(t *testing.T) { - th := New(throttleWindow) - numWindows := int32(10) + const ( + numWindows = 10 + eventPerWindow = 100 + numRoutines = 10 + ) totalDuration := throttleWindow * time.Duration(numWindows) - eventsPerWindow := 100 - interval := throttleWindow / time.Duration(eventsPerWindow) + interval := throttleWindow / time.Duration(eventPerWindow) + + th := New(throttleWindow) + ticker := time.NewTicker(interval) defer ticker.Stop() - timeCtx, cancel := context.WithTimeout(ctx, totalDuration) + ctx, cancel := context.WithTimeout(context.Background(), totalDuration) defer cancel() var callCount int32 @@ -278,14 +298,14 @@ func TestConcurrentExecuteOrSchedule(t *testing.T) { select { case <-ticker.C: // Each tick triggers multiple concurrent calls. - for i := 0; i < 10; i++ { + for range numRoutines { th.ExecuteOrSchedule(callback) } - case <-timeCtx.Done(): + case <-ctx.Done(): // Allow any trailing call to execute. time.Sleep(throttleWindow * 2) // Expect one execution per window plus one trailing call. - assert.Equal(t, numWindows+1, atomic.LoadInt32(&callCount)) + assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) return } } From 3782427c8a41dca76a87276b9f25b946214a80ec Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Mon, 24 Feb 2025 13:12:36 +0900 Subject: [PATCH 06/37] Rename package `throttle` -> `limit` - previous commit only changes directory name --- pkg/limit/throttler.go | 4 ++-- pkg/limit/throttler_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/limit/throttler.go b/pkg/limit/throttler.go index 5101bb6bf..cb447055b 100644 --- a/pkg/limit/throttler.go +++ b/pkg/limit/throttler.go @@ -14,8 +14,8 @@ * limitations under the License. */ -// Package throttle provide event timing control components -package throttle +// Package limit provide event timing control components +package limit import ( "context" diff --git a/pkg/limit/throttler_test.go b/pkg/limit/throttler_test.go index 8eddb2a48..e0b4b15fc 100644 --- a/pkg/limit/throttler_test.go +++ b/pkg/limit/throttler_test.go @@ -14,8 +14,8 @@ * limitations under the License. */ -// Package throttle provide event timing control components -package throttle +// Package limit provide event timing control components +package limit import ( "context" From f04445622cdb8b770982f8cbaa5b0f53b4256c21 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Tue, 25 Feb 2025 18:26:38 +0900 Subject: [PATCH 07/37] Rename components - `pending` -> `debouncing` - `ExecuteOrSchedule` -> `Schedule` --- pkg/limit/{throttler.go => limiter.go} | 47 +++++++++---------- .../{throttler_test.go => limiter_test.go} | 28 ++++++----- 2 files changed, 38 insertions(+), 37 deletions(-) rename pkg/limit/{throttler.go => limiter.go} (61%) rename pkg/limit/{throttler_test.go => limiter_test.go} (92%) diff --git a/pkg/limit/throttler.go b/pkg/limit/limiter.go similarity index 61% rename from pkg/limit/throttler.go rename to pkg/limit/limiter.go index cb447055b..8b814b02a 100644 --- a/pkg/limit/throttler.go +++ b/pkg/limit/limiter.go @@ -26,61 +26,60 @@ import ( "golang.org/x/time/rate" ) -// Throttler provides a combined throttling and debouncing mechanism -// that ensures eventual consistency. It calls the callback immediately -// if allowed by the rate limiter; otherwise, it schedules a trailing callback. -type Throttler struct { - lim *rate.Limiter - pending int32 // 0 means false, 1 means true +// Limiter provides a combined throttling and debouncing mechanism +// that ensures eventual consistency. +type Limiter struct { + lim *rate.Limiter + debouncing int32 // 0 means false, 1 means true } // New creates a new instance with the specified throttle intervals. -func New(window time.Duration) *Throttler { - dt := &Throttler{ - lim: rate.NewLimiter(rate.Every(window), 1), - pending: 0, +func New(window time.Duration) *Limiter { + dt := &Limiter{ + lim: rate.NewLimiter(rate.Every(window), 1), + debouncing: 0, } return dt } // Execute attempts to run the provided callback function immediately if the rate limiter allows it. // If the rate limiter does not allow immediate execution, this function blocks until the next token -// is available and then runs the callback. If there is already a pending callback, Execute returns +// is available and then runs the callback. If there is already a debouncing callback, Execute returns // immediately. This mechanism ensures that the final callback is executed after the final event, // providing eventual consistency. -func (t *Throttler) Execute(ctx context.Context, callback func() error) error { - if t.lim.Allow() { +func (l *Limiter) Execute(ctx context.Context, callback func() error) error { + if l.lim.Allow() { return callback() } - if !atomic.CompareAndSwapInt32(&t.pending, 0, 1) { + if !atomic.CompareAndSwapInt32(&l.debouncing, 0, 1) { return nil } - if err := t.lim.Wait(ctx); err != nil { + if err := l.lim.Wait(ctx); err != nil { return fmt.Errorf("wait for limiter: %w", err) } - atomic.StoreInt32(&t.pending, 0) + atomic.StoreInt32(&l.debouncing, 0) return callback() } -// ExecuteOrSchedule is the asynchronous counterpart to Execute. It runs the provided callback +// Schedule is the asynchronous counterpart to Execute. It runs the provided callback // immediately if the rate limiter allows it. Otherwise, it schedules the callback to run -// once the next token becomes available and returns immediately. If there is already a pending +// once the next token becomes available and returns immediately. If there is already a debouncing // callback, this function returns without scheduling another one. -func (t *Throttler) ExecuteOrSchedule(callback func()) { - if t.lim.Allow() { - callback() +func (l *Limiter) Schedule(callback func()) { + if l.lim.Allow() { + go callback() return } - if !atomic.CompareAndSwapInt32(&t.pending, 0, 1) { + if !atomic.CompareAndSwapInt32(&l.debouncing, 0, 1) { return } - delay := t.lim.Reserve().Delay() + delay := l.lim.Reserve().Delay() time.AfterFunc(delay, func() { - atomic.StoreInt32(&t.pending, 0) + atomic.StoreInt32(&l.debouncing, 0) callback() }) } diff --git a/pkg/limit/throttler_test.go b/pkg/limit/limiter_test.go similarity index 92% rename from pkg/limit/throttler_test.go rename to pkg/limit/limiter_test.go index e0b4b15fc..c53d23a92 100644 --- a/pkg/limit/throttler_test.go +++ b/pkg/limit/limiter_test.go @@ -49,16 +49,17 @@ func TestSynchronousExecute(t *testing.T) { t.Run("Ten consecutive calls execute callback each time", func(t *testing.T) { th := New(throttleWindow) + const numExecute = 10 var callCount int32 callback := func() error { atomic.AddInt32(&callCount, 1) return nil } - for range 10 { + for range numExecute { assert.NoError(t, th.Execute(ctx, callback)) } - assert.Equal(t, int32(10), atomic.LoadInt32(&callCount)) + assert.Equal(t, int32(numExecute), atomic.LoadInt32(&callCount)) }) } @@ -185,7 +186,7 @@ func TestContextCancellation(t *testing.T) { }) // This test verifies that when the context is canceled, - // any pending (trailing) call does not execute. + // any debouncing (trailing) call does not execute. t.Run("Trailing call fails due to context cancellation", func(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) th := New(throttleWindow) @@ -209,15 +210,15 @@ func TestContextCancellation(t *testing.T) { assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) wg.Done() }() - // Cancel the context to cancel any pending trailing call. + // Cancel the context to cancel any debouncing trailing call. cancel() wg.Wait() }) } -// TestCallAsyncDelayed verifies that if the rate limiter does not allow an immediate call, -// ExecuteOrSchedule schedules a trailing callback which eventually executes. -func TestExecuteOrSchedule(t *testing.T) { +// TestSchedule verifies that if the rate limiter does not allow an immediate call, +// Schedule schedules a trailing callback which eventually executes. +func TestSchedule(t *testing.T) { const ( throttleWindow = 10 * time.Millisecond numRoutines = 100 @@ -230,14 +231,15 @@ func TestExecuteOrSchedule(t *testing.T) { } // First call should be allowed immediately. - th.ExecuteOrSchedule(callback) + th.Schedule(callback) + time.Sleep(throttleWindow / 10) assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) // Second call should be rate limited and schedule a trailing call. before := time.Now() // it schedules only one callback for range numRoutines { - th.ExecuteOrSchedule(callback) + th.Schedule(callback) } duration := time.Since(before) assert.Less(t, duration, throttleWindow/10, "Execution time exceeded expected threshold") @@ -246,8 +248,8 @@ func TestExecuteOrSchedule(t *testing.T) { assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) } -// TestConcurrentExecuteOrSchedule verifies that the throttler behaves as expected under concurrent invocations. -func TestConcurrentExecuteOrSchedule(t *testing.T) { +// TestConcurrentSchedule verifies that the throttler behaves as expected under concurrent invocations. +func TestConcurrentSchedule(t *testing.T) { const throttleWindow = 10 * time.Millisecond t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { @@ -260,7 +262,7 @@ func TestConcurrentExecuteOrSchedule(t *testing.T) { } for range numRoutines { - th.ExecuteOrSchedule(callback) + th.Schedule(callback) } // Allow enough time for the trailing call to be scheduled. time.Sleep(throttleWindow * 2) @@ -299,7 +301,7 @@ func TestConcurrentExecuteOrSchedule(t *testing.T) { case <-ticker.C: // Each tick triggers multiple concurrent calls. for range numRoutines { - th.ExecuteOrSchedule(callback) + th.Schedule(callback) } case <-ctx.Done(): // Allow any trailing call to execute. From b9ab305c9a8b942b34f291bc4d64be6c4c80723e Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Tue, 25 Feb 2025 18:27:28 +0900 Subject: [PATCH 08/37] Change `debouncing` timing to after callback end --- pkg/limit/limiter.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index 8b814b02a..6a8958fa6 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -60,7 +60,12 @@ func (l *Limiter) Execute(ctx context.Context, callback func() error) error { return fmt.Errorf("wait for limiter: %w", err) } + if err := callback(); err != nil { + atomic.StoreInt32(&l.debouncing, 0) + return fmt.Errorf("callback: %w", err) + } atomic.StoreInt32(&l.debouncing, 0) + return callback() } @@ -79,7 +84,7 @@ func (l *Limiter) Schedule(callback func()) { } delay := l.lim.Reserve().Delay() time.AfterFunc(delay, func() { - atomic.StoreInt32(&l.debouncing, 0) callback() + atomic.StoreInt32(&l.debouncing, 0) }) } From 8bf4303f89dfa2a98eaf7604509368a207ca9299 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 26 Feb 2025 13:44:25 +0900 Subject: [PATCH 09/37] Remove Schedule --- pkg/limit/limiter.go | 22 +-------- pkg/limit/limiter_test.go | 98 --------------------------------------- 2 files changed, 1 insertion(+), 119 deletions(-) diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index 6a8958fa6..e7720e434 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -66,25 +66,5 @@ func (l *Limiter) Execute(ctx context.Context, callback func() error) error { } atomic.StoreInt32(&l.debouncing, 0) - return callback() -} - -// Schedule is the asynchronous counterpart to Execute. It runs the provided callback -// immediately if the rate limiter allows it. Otherwise, it schedules the callback to run -// once the next token becomes available and returns immediately. If there is already a debouncing -// callback, this function returns without scheduling another one. -func (l *Limiter) Schedule(callback func()) { - if l.lim.Allow() { - go callback() - return - } - - if !atomic.CompareAndSwapInt32(&l.debouncing, 0, 1) { - return - } - delay := l.lim.Reserve().Delay() - time.AfterFunc(delay, func() { - callback() - atomic.StoreInt32(&l.debouncing, 0) - }) + return nil } diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index c53d23a92..2d08213b7 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -215,101 +215,3 @@ func TestContextCancellation(t *testing.T) { wg.Wait() }) } - -// TestSchedule verifies that if the rate limiter does not allow an immediate call, -// Schedule schedules a trailing callback which eventually executes. -func TestSchedule(t *testing.T) { - const ( - throttleWindow = 10 * time.Millisecond - numRoutines = 100 - ) - th := New(throttleWindow) - - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } - - // First call should be allowed immediately. - th.Schedule(callback) - time.Sleep(throttleWindow / 10) - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - - // Second call should be rate limited and schedule a trailing call. - before := time.Now() - // it schedules only one callback - for range numRoutines { - th.Schedule(callback) - } - duration := time.Since(before) - assert.Less(t, duration, throttleWindow/10, "Execution time exceeded expected threshold") - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - time.Sleep(throttleWindow * 2) - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) -} - -// TestConcurrentSchedule verifies that the throttler behaves as expected under concurrent invocations. -func TestConcurrentSchedule(t *testing.T) { - const throttleWindow = 10 * time.Millisecond - - t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { - th := New(throttleWindow) - const numRoutines = 100 - - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } - - for range numRoutines { - th.Schedule(callback) - } - // Allow enough time for the trailing call to be scheduled. - time.Sleep(throttleWindow * 2) - // Expect exactly one immediate and one trailing callback execution. - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) - }) - - // This test simulates a continuous stream of events. - // It triggers multiple concurrent calls at a regular interval and checks that throttling - // limits the total number of callback invocations to one per window plus one trailing call. - t.Run("Throttling over continuous event stream", func(t *testing.T) { - const ( - numWindows = 10 - eventPerWindow = 100 - numRoutines = 10 - ) - totalDuration := throttleWindow * time.Duration(numWindows) - interval := throttleWindow / time.Duration(eventPerWindow) - - th := New(throttleWindow) - - ticker := time.NewTicker(interval) - defer ticker.Stop() - - ctx, cancel := context.WithTimeout(context.Background(), totalDuration) - defer cancel() - - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } - - // Continuously trigger events until the timeout. - for { - select { - case <-ticker.C: - // Each tick triggers multiple concurrent calls. - for range numRoutines { - th.Schedule(callback) - } - case <-ctx.Done(): - // Allow any trailing call to execute. - time.Sleep(throttleWindow * 2) - // Expect one execution per window plus one trailing call. - assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) - return - } - } - }) -} From 68e05c9cba87637e04a6adce1c5ad9a69840b9b8 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 26 Feb 2025 16:55:52 +0900 Subject: [PATCH 10/37] Add limit publisher --- server/backend/pubsub/publisher.go | 54 +++++++++++++++++++++++++++--- server/backend/pubsub/pubsub.go | 25 ++++++++++---- 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/server/backend/pubsub/publisher.go b/server/backend/pubsub/publisher.go index 233c7039f..78c439b58 100644 --- a/server/backend/pubsub/publisher.go +++ b/server/backend/pubsub/publisher.go @@ -18,14 +18,16 @@ package pubsub import ( + "context" "strconv" gosync "sync" "sync/atomic" - time "time" + "time" "go.uber.org/zap" "github.com/yorkie-team/yorkie/api/types/events" + "github.com/yorkie-team/yorkie/pkg/limit" "github.com/yorkie-team/yorkie/server/logging" ) @@ -38,7 +40,7 @@ func (c *loggerID) next() string { return "p" + strconv.Itoa(int(next)) } -// BatchPublisher is a publisher that publishes events in batch. +// BatchPublisher is a batchPublisher that publishes events in batch. type BatchPublisher struct { logger *zap.SugaredLogger mutex gosync.Mutex @@ -68,8 +70,6 @@ func (bp *BatchPublisher) Publish(event events.DocEvent) { bp.mutex.Lock() defer bp.mutex.Unlock() - // TODO(hackerwins): If DocumentChangedEvent is already in the batch, we don't - // need to add it again. bp.events = append(bp.events, event) } @@ -126,7 +126,51 @@ func (bp *BatchPublisher) publish() { } } -// Close stops the batch publisher +// Close stops the batch batchPublisher func (bp *BatchPublisher) Close() { close(bp.closeChan) } + +// LimitPublisher is a batchPublisher that publishes events with limit. +type LimitPublisher struct { + logger *zap.SugaredLogger + limits map[events.DocEventType]*limit.Limiter + + window time.Duration + subs *Subscriptions +} + +// NewLimitPublisher creates a new NewLimitPublisher instance. +func NewLimitPublisher(subs *Subscriptions, window time.Duration) *LimitPublisher { + lp := &LimitPublisher{ + logger: logging.New(id.next()), + limits: map[events.DocEventType]*limit.Limiter{ + events.DocChangedEvent: limit.New(window), + events.DocWatchedEvent: limit.New(window), + events.DocUnwatchedEvent: limit.New(window), + }, + window: window, + subs: subs, + } + + return lp +} + +// Publish adds the given event to the batch. If the batch is full, it publishes +// the batch. +func (lp *LimitPublisher) Publish(event events.DocEvent) { + _ = lp.limits[event.Type].Execute(context.Background(), func() error { + for _, sub := range lp.subs.Values() { + if ok := sub.Publish(event); !ok { + lp.logger.Infof( + "Publish(%s,%s) to %s timeout or closed", + event.Type, + event.Publisher, + sub.Subscriber(), + ) + } + } + + return nil + }) +} diff --git a/server/backend/pubsub/pubsub.go b/server/backend/pubsub/pubsub.go index f9a1b5e9c..a2d28dc71 100644 --- a/server/backend/pubsub/pubsub.go +++ b/server/backend/pubsub/pubsub.go @@ -32,13 +32,16 @@ import ( const ( // publishTimeout is the timeout for publishing an event. publishTimeout = 100 * gotime.Millisecond + batchWindow = 100 * gotime.Millisecond + limitWindow = 100 * gotime.Millisecond ) // Subscriptions is a map of Subscriptions. type Subscriptions struct { - docKey types.DocRefKey - internalMap *cmap.Map[string, *Subscription] - publisher *BatchPublisher + docKey types.DocRefKey + internalMap *cmap.Map[string, *Subscription] + batchPublisher *BatchPublisher + limitPublisher *LimitPublisher } func newSubscriptions(docKey types.DocRefKey) *Subscriptions { @@ -46,7 +49,8 @@ func newSubscriptions(docKey types.DocRefKey) *Subscriptions { docKey: docKey, internalMap: cmap.New[string, *Subscription](), } - s.publisher = NewBatchPublisher(s, 100*gotime.Millisecond) + s.batchPublisher = NewBatchPublisher(s, batchWindow) + s.limitPublisher = NewLimitPublisher(s, limitWindow) return s } @@ -62,7 +66,16 @@ func (s *Subscriptions) Values() []*Subscription { // Publish publishes the given event. func (s *Subscriptions) Publish(event events.DocEvent) { - s.publisher.Publish(event) + switch event.Type { + case events.DocBroadcastEvent: + s.batchPublisher.Publish(event) + case events.DocChangedEvent: + s.limitPublisher.Publish(event) + case events.DocWatchedEvent: + s.limitPublisher.Publish(event) + case events.DocUnwatchedEvent: + s.limitPublisher.Publish(event) + } } // Delete deletes the subscription of the given id. @@ -82,7 +95,7 @@ func (s *Subscriptions) Len() int { // Close closes the subscriptions. func (s *Subscriptions) Close() { - s.publisher.Close() + s.batchPublisher.Close() } // PubSub is the memory implementation of PubSub, used for single server. From 1cd6410b79146fa7ef206628510a30bec035580d Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 26 Feb 2025 16:58:55 +0900 Subject: [PATCH 11/37] Filter self produced event in client --- client/client.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/client/client.go b/client/client.go index e36ba8e3c..121294da2 100644 --- a/client/client.go +++ b/client/client.go @@ -510,7 +510,7 @@ func (c *Client) runWatchLoop( if !stream.Receive() { return ErrInitializationNotReceived } - if _, err := handleResponse(stream.Msg(), doc); err != nil { + if _, err := handleResponse(stream.Msg(), doc, c.id); err != nil { return err } if err = stream.Err(); err != nil { @@ -523,7 +523,7 @@ func (c *Client) runWatchLoop( go func() { for stream.Receive() { pbResp := stream.Msg() - resp, err := handleResponse(pbResp, doc) + resp, err := handleResponse(pbResp, doc, c.id) if err != nil { rch <- WatchResponse{Err: err} ctx.Done() @@ -594,16 +594,17 @@ func (c *Client) runWatchLoop( func handleResponse( pbResp *api.WatchDocumentResponse, doc *document.Document, + id *time.ActorID, ) (*WatchResponse, error) { switch resp := pbResp.Body.(type) { case *api.WatchDocumentResponse_Initialization_: var clientIDs []string for _, clientID := range resp.Initialization.ClientIds { - id, err := time.ActorIDFromHex(clientID) + cli, err := time.ActorIDFromHex(clientID) if err != nil { return nil, err } - clientIDs = append(clientIDs, id.String()) + clientIDs = append(clientIDs, cli.String()) } doc.SetOnlineClients(clientIDs...) @@ -619,6 +620,10 @@ func handleResponse( return nil, err } + if cli.Compare(id) == 0 { + return nil, nil + } + switch eventType { case events.DocChangedEvent: return &WatchResponse{Type: DocumentChanged}, nil From 9e73b0a026aa4dc1f67fbac64dd1f0c9ecc373ad Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Mon, 3 Mar 2025 19:20:58 +0900 Subject: [PATCH 12/37] Revert --- client/client.go | 13 ++-- pkg/limit/limiter.go | 22 ++++++- pkg/limit/limiter_test.go | 98 ++++++++++++++++++++++++++++++ server/backend/pubsub/publisher.go | 54 ++-------------- server/backend/pubsub/pubsub.go | 25 ++------ 5 files changed, 134 insertions(+), 78 deletions(-) diff --git a/client/client.go b/client/client.go index 121294da2..e36ba8e3c 100644 --- a/client/client.go +++ b/client/client.go @@ -510,7 +510,7 @@ func (c *Client) runWatchLoop( if !stream.Receive() { return ErrInitializationNotReceived } - if _, err := handleResponse(stream.Msg(), doc, c.id); err != nil { + if _, err := handleResponse(stream.Msg(), doc); err != nil { return err } if err = stream.Err(); err != nil { @@ -523,7 +523,7 @@ func (c *Client) runWatchLoop( go func() { for stream.Receive() { pbResp := stream.Msg() - resp, err := handleResponse(pbResp, doc, c.id) + resp, err := handleResponse(pbResp, doc) if err != nil { rch <- WatchResponse{Err: err} ctx.Done() @@ -594,17 +594,16 @@ func (c *Client) runWatchLoop( func handleResponse( pbResp *api.WatchDocumentResponse, doc *document.Document, - id *time.ActorID, ) (*WatchResponse, error) { switch resp := pbResp.Body.(type) { case *api.WatchDocumentResponse_Initialization_: var clientIDs []string for _, clientID := range resp.Initialization.ClientIds { - cli, err := time.ActorIDFromHex(clientID) + id, err := time.ActorIDFromHex(clientID) if err != nil { return nil, err } - clientIDs = append(clientIDs, cli.String()) + clientIDs = append(clientIDs, id.String()) } doc.SetOnlineClients(clientIDs...) @@ -620,10 +619,6 @@ func handleResponse( return nil, err } - if cli.Compare(id) == 0 { - return nil, nil - } - switch eventType { case events.DocChangedEvent: return &WatchResponse{Type: DocumentChanged}, nil diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index e7720e434..6a8958fa6 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -66,5 +66,25 @@ func (l *Limiter) Execute(ctx context.Context, callback func() error) error { } atomic.StoreInt32(&l.debouncing, 0) - return nil + return callback() +} + +// Schedule is the asynchronous counterpart to Execute. It runs the provided callback +// immediately if the rate limiter allows it. Otherwise, it schedules the callback to run +// once the next token becomes available and returns immediately. If there is already a debouncing +// callback, this function returns without scheduling another one. +func (l *Limiter) Schedule(callback func()) { + if l.lim.Allow() { + go callback() + return + } + + if !atomic.CompareAndSwapInt32(&l.debouncing, 0, 1) { + return + } + delay := l.lim.Reserve().Delay() + time.AfterFunc(delay, func() { + callback() + atomic.StoreInt32(&l.debouncing, 0) + }) } diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index 2d08213b7..c53d23a92 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -215,3 +215,101 @@ func TestContextCancellation(t *testing.T) { wg.Wait() }) } + +// TestSchedule verifies that if the rate limiter does not allow an immediate call, +// Schedule schedules a trailing callback which eventually executes. +func TestSchedule(t *testing.T) { + const ( + throttleWindow = 10 * time.Millisecond + numRoutines = 100 + ) + th := New(throttleWindow) + + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + // First call should be allowed immediately. + th.Schedule(callback) + time.Sleep(throttleWindow / 10) + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + + // Second call should be rate limited and schedule a trailing call. + before := time.Now() + // it schedules only one callback + for range numRoutines { + th.Schedule(callback) + } + duration := time.Since(before) + assert.Less(t, duration, throttleWindow/10, "Execution time exceeded expected threshold") + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + time.Sleep(throttleWindow * 2) + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) +} + +// TestConcurrentSchedule verifies that the throttler behaves as expected under concurrent invocations. +func TestConcurrentSchedule(t *testing.T) { + const throttleWindow = 10 * time.Millisecond + + t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { + th := New(throttleWindow) + const numRoutines = 100 + + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + for range numRoutines { + th.Schedule(callback) + } + // Allow enough time for the trailing call to be scheduled. + time.Sleep(throttleWindow * 2) + // Expect exactly one immediate and one trailing callback execution. + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + }) + + // This test simulates a continuous stream of events. + // It triggers multiple concurrent calls at a regular interval and checks that throttling + // limits the total number of callback invocations to one per window plus one trailing call. + t.Run("Throttling over continuous event stream", func(t *testing.T) { + const ( + numWindows = 10 + eventPerWindow = 100 + numRoutines = 10 + ) + totalDuration := throttleWindow * time.Duration(numWindows) + interval := throttleWindow / time.Duration(eventPerWindow) + + th := New(throttleWindow) + + ticker := time.NewTicker(interval) + defer ticker.Stop() + + ctx, cancel := context.WithTimeout(context.Background(), totalDuration) + defer cancel() + + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + // Continuously trigger events until the timeout. + for { + select { + case <-ticker.C: + // Each tick triggers multiple concurrent calls. + for range numRoutines { + th.Schedule(callback) + } + case <-ctx.Done(): + // Allow any trailing call to execute. + time.Sleep(throttleWindow * 2) + // Expect one execution per window plus one trailing call. + assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) + return + } + } + }) +} diff --git a/server/backend/pubsub/publisher.go b/server/backend/pubsub/publisher.go index 78c439b58..233c7039f 100644 --- a/server/backend/pubsub/publisher.go +++ b/server/backend/pubsub/publisher.go @@ -18,16 +18,14 @@ package pubsub import ( - "context" "strconv" gosync "sync" "sync/atomic" - "time" + time "time" "go.uber.org/zap" "github.com/yorkie-team/yorkie/api/types/events" - "github.com/yorkie-team/yorkie/pkg/limit" "github.com/yorkie-team/yorkie/server/logging" ) @@ -40,7 +38,7 @@ func (c *loggerID) next() string { return "p" + strconv.Itoa(int(next)) } -// BatchPublisher is a batchPublisher that publishes events in batch. +// BatchPublisher is a publisher that publishes events in batch. type BatchPublisher struct { logger *zap.SugaredLogger mutex gosync.Mutex @@ -70,6 +68,8 @@ func (bp *BatchPublisher) Publish(event events.DocEvent) { bp.mutex.Lock() defer bp.mutex.Unlock() + // TODO(hackerwins): If DocumentChangedEvent is already in the batch, we don't + // need to add it again. bp.events = append(bp.events, event) } @@ -126,51 +126,7 @@ func (bp *BatchPublisher) publish() { } } -// Close stops the batch batchPublisher +// Close stops the batch publisher func (bp *BatchPublisher) Close() { close(bp.closeChan) } - -// LimitPublisher is a batchPublisher that publishes events with limit. -type LimitPublisher struct { - logger *zap.SugaredLogger - limits map[events.DocEventType]*limit.Limiter - - window time.Duration - subs *Subscriptions -} - -// NewLimitPublisher creates a new NewLimitPublisher instance. -func NewLimitPublisher(subs *Subscriptions, window time.Duration) *LimitPublisher { - lp := &LimitPublisher{ - logger: logging.New(id.next()), - limits: map[events.DocEventType]*limit.Limiter{ - events.DocChangedEvent: limit.New(window), - events.DocWatchedEvent: limit.New(window), - events.DocUnwatchedEvent: limit.New(window), - }, - window: window, - subs: subs, - } - - return lp -} - -// Publish adds the given event to the batch. If the batch is full, it publishes -// the batch. -func (lp *LimitPublisher) Publish(event events.DocEvent) { - _ = lp.limits[event.Type].Execute(context.Background(), func() error { - for _, sub := range lp.subs.Values() { - if ok := sub.Publish(event); !ok { - lp.logger.Infof( - "Publish(%s,%s) to %s timeout or closed", - event.Type, - event.Publisher, - sub.Subscriber(), - ) - } - } - - return nil - }) -} diff --git a/server/backend/pubsub/pubsub.go b/server/backend/pubsub/pubsub.go index a2d28dc71..f9a1b5e9c 100644 --- a/server/backend/pubsub/pubsub.go +++ b/server/backend/pubsub/pubsub.go @@ -32,16 +32,13 @@ import ( const ( // publishTimeout is the timeout for publishing an event. publishTimeout = 100 * gotime.Millisecond - batchWindow = 100 * gotime.Millisecond - limitWindow = 100 * gotime.Millisecond ) // Subscriptions is a map of Subscriptions. type Subscriptions struct { - docKey types.DocRefKey - internalMap *cmap.Map[string, *Subscription] - batchPublisher *BatchPublisher - limitPublisher *LimitPublisher + docKey types.DocRefKey + internalMap *cmap.Map[string, *Subscription] + publisher *BatchPublisher } func newSubscriptions(docKey types.DocRefKey) *Subscriptions { @@ -49,8 +46,7 @@ func newSubscriptions(docKey types.DocRefKey) *Subscriptions { docKey: docKey, internalMap: cmap.New[string, *Subscription](), } - s.batchPublisher = NewBatchPublisher(s, batchWindow) - s.limitPublisher = NewLimitPublisher(s, limitWindow) + s.publisher = NewBatchPublisher(s, 100*gotime.Millisecond) return s } @@ -66,16 +62,7 @@ func (s *Subscriptions) Values() []*Subscription { // Publish publishes the given event. func (s *Subscriptions) Publish(event events.DocEvent) { - switch event.Type { - case events.DocBroadcastEvent: - s.batchPublisher.Publish(event) - case events.DocChangedEvent: - s.limitPublisher.Publish(event) - case events.DocWatchedEvent: - s.limitPublisher.Publish(event) - case events.DocUnwatchedEvent: - s.limitPublisher.Publish(event) - } + s.publisher.Publish(event) } // Delete deletes the subscription of the given id. @@ -95,7 +82,7 @@ func (s *Subscriptions) Len() int { // Close closes the subscriptions. func (s *Subscriptions) Close() { - s.batchPublisher.Close() + s.publisher.Close() } // PubSub is the memory implementation of PubSub, used for single server. From 90114a756e145a1e5235d7f8f8bbb243acaf9d32 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Mon, 3 Mar 2025 19:53:06 +0900 Subject: [PATCH 13/37] Remove limiter package --- pkg/limit/limiter.go | 68 ++------ pkg/limit/limiter_test.go | 315 -------------------------------------- 2 files changed, 10 insertions(+), 373 deletions(-) delete mode 100644 pkg/limit/limiter_test.go diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index 6a8958fa6..e77f67593 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -18,73 +18,25 @@ package limit import ( - "context" - "fmt" - "sync/atomic" - "time" - "golang.org/x/time/rate" ) -// Limiter provides a combined throttling and debouncing mechanism -// that ensures eventual consistency. -type Limiter struct { - lim *rate.Limiter - debouncing int32 // 0 means false, 1 means true -} - -// New creates a new instance with the specified throttle intervals. -func New(window time.Duration) *Limiter { - dt := &Limiter{ - lim: rate.NewLimiter(rate.Every(window), 1), - debouncing: 0, - } - return dt +type Limiter[E any] struct { + lim *rate.Limiter + event E + callback func(E) error } -// Execute attempts to run the provided callback function immediately if the rate limiter allows it. -// If the rate limiter does not allow immediate execution, this function blocks until the next token -// is available and then runs the callback. If there is already a debouncing callback, Execute returns -// immediately. This mechanism ensures that the final callback is executed after the final event, -// providing eventual consistency. -func (l *Limiter) Execute(ctx context.Context, callback func() error) error { +func (l Limiter[E]) Execute(e E) error { if l.lim.Allow() { - return callback() + return l.callback(e) } - if !atomic.CompareAndSwapInt32(&l.debouncing, 0, 1) { - return nil - } - - if err := l.lim.Wait(ctx); err != nil { - return fmt.Errorf("wait for limiter: %w", err) - } + l.event = e - if err := callback(); err != nil { - atomic.StoreInt32(&l.debouncing, 0) - return fmt.Errorf("callback: %w", err) - } - atomic.StoreInt32(&l.debouncing, 0) - - return callback() + return nil } -// Schedule is the asynchronous counterpart to Execute. It runs the provided callback -// immediately if the rate limiter allows it. Otherwise, it schedules the callback to run -// once the next token becomes available and returns immediately. If there is already a debouncing -// callback, this function returns without scheduling another one. -func (l *Limiter) Schedule(callback func()) { - if l.lim.Allow() { - go callback() - return - } - - if !atomic.CompareAndSwapInt32(&l.debouncing, 0, 1) { - return - } - delay := l.lim.Reserve().Delay() - time.AfterFunc(delay, func() { - callback() - atomic.StoreInt32(&l.debouncing, 0) - }) +func (l Limiter[E]) Flush() error { + return l.callback(l.event) } diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go deleted file mode 100644 index c53d23a92..000000000 --- a/pkg/limit/limiter_test.go +++ /dev/null @@ -1,315 +0,0 @@ -/* - * Copyright 2025 The Yorkie Authors. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// Package limit provide event timing control components -package limit - -import ( - "context" - "errors" - "sync" - "sync/atomic" - "testing" - "time" - - "github.com/stretchr/testify/assert" -) - -// TestSynchronousExecute verifies that synchronous calls to the throttler -// execute the callback immediately without any delays. -func TestSynchronousExecute(t *testing.T) { - ctx := context.Background() - const throttleWindow = 10 * time.Millisecond - - t.Run("Single call executes callback exactly once", func(t *testing.T) { - th := New(throttleWindow) - var callCount int32 - callback := func() error { - atomic.AddInt32(&callCount, 1) - return nil - } - - err := th.Execute(ctx, callback) - assert.NoError(t, err) - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - }) - - t.Run("Ten consecutive calls execute callback each time", func(t *testing.T) { - th := New(throttleWindow) - const numExecute = 10 - var callCount int32 - callback := func() error { - atomic.AddInt32(&callCount, 1) - return nil - } - - for range numExecute { - assert.NoError(t, th.Execute(ctx, callback)) - } - assert.Equal(t, int32(numExecute), atomic.LoadInt32(&callCount)) - }) -} - -// TestConcurrentExecute verifies that the throttler behaves as expected under concurrent invocations. -func TestConcurrentExecute(t *testing.T) { - ctx := context.Background() - const throttleWindow = 100 * time.Millisecond - - // In this test, many concurrent calls are made. The throttler should execute one immediate call - // and then schedule a trailing call, resulting in exactly two executions. - t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { - th := New(throttleWindow) - const numRoutines = 1000 - var callCount int32 - callback := func() error { - atomic.AddInt32(&callCount, 1) - return nil - } - - var wg sync.WaitGroup - wg.Add(numRoutines) - - for range numRoutines { - go func() { - assert.NoError(t, th.Execute(ctx, callback)) - wg.Done() - }() - } - - wg.Wait() - // Expect exactly one immediate and one trailing callback execution. - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) - }) - - // This test simulates a continuous stream of events. - // It triggers multiple concurrent calls at a regular interval and checks that throttling - // limits the total number of callback invocations to one per window plus one trailing call. - t.Run("Throttling over continuous event stream", func(t *testing.T) { - const ( - numWindows = 10 - eventPerWindow = 100 - numRoutines = 10 - ) - totalDuration := throttleWindow * time.Duration(numWindows) - interval := throttleWindow / time.Duration(eventPerWindow) - - th := New(throttleWindow) - - ticker := time.NewTicker(interval) - defer ticker.Stop() - - timeCtx, cancel := context.WithTimeout(ctx, totalDuration) - defer cancel() - - var callCount int32 - callback := func() error { - atomic.AddInt32(&callCount, 1) - return nil - } - - // Continuously trigger events until the timeout. - for { - select { - case <-ticker.C: - // Each tick triggers multiple concurrent calls. - for range numRoutines { - go func() { - assert.NoError(t, th.Execute(ctx, callback)) - }() - } - case <-timeCtx.Done(): - // Allow any trailing call to execute. - time.Sleep(throttleWindow) - // Expect one execution per window plus one trailing call. - assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) - return - } - } - }) -} - -// TestCallbackErrorPropagation checks that errors returned by the callback -// are immediately propagated back to the caller. -func TestCallbackErrorPropagation(t *testing.T) { - ctx := context.Background() - const throttleWindow = 10 * time.Millisecond - expectedErr := errors.New("callback error") - - t.Run("Immediate callback error is propagated", func(t *testing.T) { - th := New(throttleWindow) - callback := func() error { - return expectedErr - } - err := th.Execute(ctx, callback) - assert.ErrorIs(t, err, expectedErr) - }) -} - -// TestContextCancellation verifies the throttler's behavior when the context -// expires (deadline exceeded) or is canceled. -func TestContextCancellation(t *testing.T) { - const throttleWindow = 10 * time.Millisecond - - // In this test the context deadline is shorter than the throttle window. - // The trailing call should fail with a deadline exceeded error. - t.Run("Trailing call fails due to context deadline exceeded", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), throttleWindow/2) - defer cancel() - th := New(throttleWindow) - var callCount int32 - callback := func() error { - atomic.AddInt32(&callCount, 1) - return nil - } - - // The first call executes immediately. - assert.NoError(t, th.Execute(ctx, callback)) - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - // The second call is delayed and should eventually time out. - err := th.Execute(ctx, callback) - assert.ErrorAs(t, err, &context.DeadlineExceeded) - // Ensure the callback was not executed a second time. - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - }) - - // This test verifies that when the context is canceled, - // any debouncing (trailing) call does not execute. - t.Run("Trailing call fails due to context cancellation", func(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - th := New(throttleWindow) - var callCount int32 - callback := func() error { - atomic.AddInt32(&callCount, 1) - return nil - } - - // First call executes immediately. - assert.NoError(t, th.Execute(ctx, callback)) - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - // Launch a trailing call that will be affected by cancellation. - var wg sync.WaitGroup - wg.Add(1) - - go func() { - err := th.Execute(ctx, callback) - assert.ErrorAs(t, err, &context.Canceled) - // Verify that the trailing call was not executed. - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - wg.Done() - }() - // Cancel the context to cancel any debouncing trailing call. - cancel() - wg.Wait() - }) -} - -// TestSchedule verifies that if the rate limiter does not allow an immediate call, -// Schedule schedules a trailing callback which eventually executes. -func TestSchedule(t *testing.T) { - const ( - throttleWindow = 10 * time.Millisecond - numRoutines = 100 - ) - th := New(throttleWindow) - - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } - - // First call should be allowed immediately. - th.Schedule(callback) - time.Sleep(throttleWindow / 10) - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - - // Second call should be rate limited and schedule a trailing call. - before := time.Now() - // it schedules only one callback - for range numRoutines { - th.Schedule(callback) - } - duration := time.Since(before) - assert.Less(t, duration, throttleWindow/10, "Execution time exceeded expected threshold") - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) - time.Sleep(throttleWindow * 2) - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) -} - -// TestConcurrentSchedule verifies that the throttler behaves as expected under concurrent invocations. -func TestConcurrentSchedule(t *testing.T) { - const throttleWindow = 10 * time.Millisecond - - t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { - th := New(throttleWindow) - const numRoutines = 100 - - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } - - for range numRoutines { - th.Schedule(callback) - } - // Allow enough time for the trailing call to be scheduled. - time.Sleep(throttleWindow * 2) - // Expect exactly one immediate and one trailing callback execution. - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) - }) - - // This test simulates a continuous stream of events. - // It triggers multiple concurrent calls at a regular interval and checks that throttling - // limits the total number of callback invocations to one per window plus one trailing call. - t.Run("Throttling over continuous event stream", func(t *testing.T) { - const ( - numWindows = 10 - eventPerWindow = 100 - numRoutines = 10 - ) - totalDuration := throttleWindow * time.Duration(numWindows) - interval := throttleWindow / time.Duration(eventPerWindow) - - th := New(throttleWindow) - - ticker := time.NewTicker(interval) - defer ticker.Stop() - - ctx, cancel := context.WithTimeout(context.Background(), totalDuration) - defer cancel() - - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } - - // Continuously trigger events until the timeout. - for { - select { - case <-ticker.C: - // Each tick triggers multiple concurrent calls. - for range numRoutines { - th.Schedule(callback) - } - case <-ctx.Done(): - // Allow any trailing call to execute. - time.Sleep(throttleWindow * 2) - // Expect one execution per window plus one trailing call. - assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) - return - } - } - }) -} From ac199068e0cccf749076bafe10908b8b1bde0035 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 5 Mar 2025 15:34:13 +0900 Subject: [PATCH 14/37] Refactor limiter --- pkg/limit/bucket.go | 26 +++++++++ pkg/limit/limiter.go | 134 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 147 insertions(+), 13 deletions(-) create mode 100644 pkg/limit/bucket.go diff --git a/pkg/limit/bucket.go b/pkg/limit/bucket.go new file mode 100644 index 000000000..d663c3cdf --- /dev/null +++ b/pkg/limit/bucket.go @@ -0,0 +1,26 @@ +package limit + +import "time" + +// Bucket is one token bucket that charged every window +type Bucket struct { + window time.Duration + // last is the last time the limiter's tokens field was updated + last time.Time +} + +func NewBucket(now time.Time, window time.Duration) Bucket { + return Bucket{ + window: window, + last: now, + } +} + +func (b *Bucket) Allow(now time.Time) bool { + if now.Before(b.last.Add(b.window)) { + return false + } + + b.last = now + return true +} diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index e77f67593..cc2140a97 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -14,29 +14,137 @@ * limitations under the License. */ -// Package limit provide event timing control components +// Package limit provides event timing control components. package limit import ( - "golang.org/x/time/rate" + "container/list" + "sync" + "time" ) -type Limiter[E any] struct { - lim *rate.Limiter - event E - callback func(E) error +// Limiter is a cache that ensures the most recently accessed keys have a TTL +// beyond which keys are forcibly expired. +type Limiter[K comparable] struct { + mu sync.Mutex + closeChan chan struct{} + + expireInterval time.Duration + bucketTTL time.Duration + limitWindow time.Duration + + evictionList *list.List + limiters map[K]*list.Element +} + +// New creates a new Limiter with the specified expiration interval, TTL, and rate limit window. +func New[K comparable](expireInterval, ttl, limitWindow time.Duration) *Limiter[K] { + lim := &Limiter[K]{ + closeChan: make(chan struct{}), + expireInterval: expireInterval, + bucketTTL: ttl, + limitWindow: limitWindow, + evictionList: list.New(), + limiters: make(map[K]*list.Element), + } + + // Start the expiration process in a separate goroutine. + go lim.processLoop() + return lim } -func (l Limiter[E]) Execute(e E) error { - if l.lim.Allow() { - return l.callback(e) +// limitEntry holds the rate limiter and its expiration time for a given key. +type limitEntry[K comparable] struct { + key K + bucket Bucket + expireTime time.Time + debouncingCallback func() +} + +// Allow checks if an event for the given key is allowed according to the rate limiter. +// If the event is not allowed, the provided callback is stored for debouncing and later execution upon eviction. +func (l *Limiter[K]) Allow(key K, callback func()) bool { + l.mu.Lock() + defer l.mu.Unlock() + + now := time.Now() + if elem, exists := l.limiters[key]; exists { + entry := elem.Value.(*limitEntry[K]) + allowed := entry.bucket.Allow(now) + // If allowed, clear any pending debouncing callback; if not, store the new callback. + if allowed { + entry.debouncingCallback = nil + } else { + entry.debouncingCallback = callback + } + + // Update recency and extend TTL. + l.evictionList.MoveToFront(elem) + entry.expireTime = now.Add(l.bucketTTL) + return allowed } - l.event = e + // Create a new rate bucket for the key. + bucket := NewBucket(now, l.limitWindow) - return nil + entry := &limitEntry[K]{ + key: key, + bucket: bucket, + expireTime: now.Add(l.bucketTTL), + } + elem := l.evictionList.PushFront(entry) + l.limiters[key] = elem + return true +} + +// processLoop periodically calls expire to remove expired entries. +func (l *Limiter[K]) processLoop() { + ticker := time.NewTicker(l.expireInterval) + defer ticker.Stop() + + for { + select { + case <-ticker.C: + l.expire() + case <-l.closeChan: + return + } + } +} + +// expire removes entries from the eviction list that have passed their expiration time +// and triggers their debouncing callbacks if present. +func (l *Limiter[K]) expire() { + now := time.Now() + expiredEntries := make([]*limitEntry[K], 0, 100) + + l.mu.Lock() + // Remove all expired entries from the back of the eviction list. + for { + elem := l.evictionList.Back() + if elem == nil { + break + } + + entry := elem.Value.(*limitEntry[K]) + if now.Before(entry.expireTime) { + break + } + + expiredEntries = append(expiredEntries, entry) + l.evictionList.Remove(elem) + delete(l.limiters, entry.key) + } + l.mu.Unlock() + + for _, entry := range expiredEntries { + if entry.debouncingCallback != nil { + entry.debouncingCallback() + } + } } -func (l Limiter[E]) Flush() error { - return l.callback(l.event) +// Close stops the process loop and cleans up resources. +func (l *Limiter[K]) Close() { + close(l.closeChan) } From 7e7efc0f8e8184010d4b9ff7469f4b20ff82720f Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Thu, 6 Mar 2025 13:13:47 +0900 Subject: [PATCH 15/37] Add webhook manager component --- api/types/event_webhook.go | 36 ++++++++++++ api/types/resource_ref_key.go | 9 +++ pkg/limit/limiter.go | 90 ++++++++++++++++-------------- server/backend/backend.go | 25 +++++---- server/backend/webhook/manager.go | 92 +++++++++++++++++++++++++++++++ server/packs/packs.go | 20 ++++--- server/webhook/events.go | 92 ------------------------------- 7 files changed, 210 insertions(+), 154 deletions(-) create mode 100644 server/backend/webhook/manager.go delete mode 100644 server/webhook/events.go diff --git a/api/types/event_webhook.go b/api/types/event_webhook.go index 21d70a54c..9a1c6f394 100644 --- a/api/types/event_webhook.go +++ b/api/types/event_webhook.go @@ -16,6 +16,14 @@ package types +import ( + "encoding/json" + "fmt" + "time" +) + +const DateFormat = "2006-01-02T15:04:05.000Z" + // EventWebhookType represents event webhook type type EventWebhookType string @@ -40,3 +48,31 @@ type EventWebhookRequest struct { Type EventWebhookType `json:"type"` Attributes EventWebhookAttribute `json:"attributes"` } + +// NewRequestBody builds the request body for the event webhook. +func NewRequestBody(docKey string, event EventWebhookType) ([]byte, error) { + req := EventWebhookRequest{ + Type: event, + Attributes: EventWebhookAttribute{ + Key: docKey, + IssuedAt: time.Now().UTC().Format(DateFormat), + }, + } + + body, err := json.Marshal(req) + if err != nil { + return nil, fmt.Errorf("marshal event webhook request: %w", err) + } + return body, nil +} + +type EventWebhookInfo struct { + EventRefKey EventRefKey + Attribute WebhookAttribute +} + +type WebhookAttribute struct { + SigningKey string + URL string + DocKey string +} diff --git a/api/types/resource_ref_key.go b/api/types/resource_ref_key.go index d295d829a..1c036e0f6 100644 --- a/api/types/resource_ref_key.go +++ b/api/types/resource_ref_key.go @@ -52,3 +52,12 @@ type SnapshotRefKey struct { func (r SnapshotRefKey) String() string { return fmt.Sprintf("Snapshot (%s.%s.%d)", r.ProjectID, r.DocID, r.ServerSeq) } + +type EventRefKey struct { + DocRefKey + EventWebhookType +} + +func (r EventRefKey) String() string { + return fmt.Sprintf("DocEvent (%s.%s.%d)", r.ProjectID, r.DocID, r.EventWebhookType) +} diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index cc2140a97..f374a81dd 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -14,7 +14,7 @@ * limitations under the License. */ -// Package limit provides event timing control components. +// Package limit provides rate-limiting functionality with debouncing support. package limit import ( @@ -23,128 +23,134 @@ import ( "time" ) -// Limiter is a cache that ensures the most recently accessed keys have a TTL -// beyond which keys are forcibly expired. +// Limiter provides rate limiting functionality with a debouncing callback. +// It maintains a single token bucket. type Limiter[K comparable] struct { mu sync.Mutex closeChan chan struct{} expireInterval time.Duration - bucketTTL time.Duration - limitWindow time.Duration + rateWindow time.Duration + entryTTL time.Duration + // evictionList holds the limiter entries in order of recency. evictionList *list.List - limiters map[K]*list.Element + // entries maps keys to their corresponding list element for quick lookup. + entries map[K]*list.Element } -// New creates a new Limiter with the specified expiration interval, TTL, and rate limit window. -func New[K comparable](expireInterval, ttl, limitWindow time.Duration) *Limiter[K] { +// New creates and returns a new Limiter instance. +// Parameters: +// +// expireInterval: How often to check for expired entries. +// rateWindow: The time window for rate limiting. +// entryTTL: The time-to-live for each rate bucket entry. +func New[K comparable](expireInterval, rateWindow, entryTTL time.Duration) *Limiter[K] { lim := &Limiter[K]{ closeChan: make(chan struct{}), expireInterval: expireInterval, - bucketTTL: ttl, - limitWindow: limitWindow, + rateWindow: rateWindow, + entryTTL: entryTTL, evictionList: list.New(), - limiters: make(map[K]*list.Element), + entries: make(map[K]*list.Element), } - // Start the expiration process in a separate goroutine. - go lim.processLoop() + // Start the background expiration process. + go lim.expirationLoop() return lim } -// limitEntry holds the rate limiter and its expiration time for a given key. -type limitEntry[K comparable] struct { +// limiterEntry represents an entry in the Limiter for a specific key. +type limiterEntry[K comparable] struct { key K bucket Bucket expireTime time.Time - debouncingCallback func() + debouncingCallback func() error } -// Allow checks if an event for the given key is allowed according to the rate limiter. -// If the event is not allowed, the provided callback is stored for debouncing and later execution upon eviction. -func (l *Limiter[K]) Allow(key K, callback func()) bool { +// Allow checks if an event is allowed for the given key based on the rate bucket. +// If allowed, it clears any pending debouncing callback; otherwise, it stores the provided callback. +// It returns true if the event is allowed immediately. +func (l *Limiter[K]) Allow(key K, callback func() error) bool { l.mu.Lock() defer l.mu.Unlock() now := time.Now() - if elem, exists := l.limiters[key]; exists { - entry := elem.Value.(*limitEntry[K]) + if elem, exists := l.entries[key]; exists { + entry := elem.Value.(*limiterEntry[K]) allowed := entry.bucket.Allow(now) - // If allowed, clear any pending debouncing callback; if not, store the new callback. if allowed { entry.debouncingCallback = nil } else { entry.debouncingCallback = callback } - // Update recency and extend TTL. l.evictionList.MoveToFront(elem) - entry.expireTime = now.Add(l.bucketTTL) + entry.expireTime = now.Add(l.entryTTL) return allowed } - // Create a new rate bucket for the key. - bucket := NewBucket(now, l.limitWindow) - - entry := &limitEntry[K]{ + // Create a new rate bucket for a new key. + bucket := NewBucket(now, l.rateWindow) + entry := &limiterEntry[K]{ key: key, bucket: bucket, - expireTime: now.Add(l.bucketTTL), + expireTime: now.Add(l.entryTTL), } elem := l.evictionList.PushFront(entry) - l.limiters[key] = elem + l.entries[key] = elem return true } -// processLoop periodically calls expire to remove expired entries. -func (l *Limiter[K]) processLoop() { +// expirationLoop runs in a separate goroutine to periodically remove expired entries. +func (l *Limiter[K]) expirationLoop() { ticker := time.NewTicker(l.expireInterval) defer ticker.Stop() for { select { case <-ticker.C: - l.expire() + l.expireEntries() case <-l.closeChan: return } } } -// expire removes entries from the eviction list that have passed their expiration time -// and triggers their debouncing callbacks if present. -func (l *Limiter[K]) expire() { +// expireEntries checks for and removes expired entries from the limiter. +// It also triggers any stored debouncing callbacks for expired entries. +func (l *Limiter[K]) expireEntries() { now := time.Now() - expiredEntries := make([]*limitEntry[K], 0, 100) + expiredEntries := make([]*limiterEntry[K], 0, 100) l.mu.Lock() - // Remove all expired entries from the back of the eviction list. for { elem := l.evictionList.Back() if elem == nil { break } - entry := elem.Value.(*limitEntry[K]) + entry := elem.Value.(*limiterEntry[K]) if now.Before(entry.expireTime) { break } expiredEntries = append(expiredEntries, entry) l.evictionList.Remove(elem) - delete(l.limiters, entry.key) + delete(l.entries, entry.key) } l.mu.Unlock() + // Process debouncing callbacks for expired entries. for _, entry := range expiredEntries { if entry.debouncingCallback != nil { - entry.debouncingCallback() + if err := entry.debouncingCallback(); err != nil { + } } } } -// Close stops the process loop and cleans up resources. +// Close terminates the expiration loop and cleans up resources. func (l *Limiter[K]) Close() { close(l.closeChan) } diff --git a/server/backend/backend.go b/server/backend/backend.go index bc707d9c9..9cfb64103 100644 --- a/server/backend/backend.go +++ b/server/backend/backend.go @@ -22,12 +22,13 @@ package backend import ( "context" "fmt" + "github.com/yorkie-team/yorkie/server/backend/webhook" "os" "github.com/yorkie-team/yorkie/api/types" "github.com/yorkie-team/yorkie/pkg/cache" pkgtypes "github.com/yorkie-team/yorkie/pkg/types" - "github.com/yorkie-team/yorkie/pkg/webhook" + pkgwebhook "github.com/yorkie-team/yorkie/pkg/webhook" "github.com/yorkie-team/yorkie/server/backend/background" "github.com/yorkie-team/yorkie/server/backend/database" memdb "github.com/yorkie-team/yorkie/server/backend/database/memory" @@ -51,10 +52,10 @@ type Backend struct { *types.AuthWebhookResponse, ]] // AuthWebhookClient is used to send auth webhook. - AuthWebhookClient *webhook.Client[types.AuthWebhookRequest, types.AuthWebhookResponse] + AuthWebhookClient *pkgwebhook.Client[types.AuthWebhookRequest, types.AuthWebhookResponse] - // EventWebhookClient is used to send event webhook - EventWebhookClient *webhook.Client[types.EventWebhookRequest, int] + // EventWebhookManager is used to send event webhook + EventWebhookManager *webhook.Manager // PubSub is used to publish/subscribe events to/from clients. PubSub *pubsub.PubSub @@ -97,8 +98,8 @@ func New( authWebhookCache := cache.NewLRUExpireCache[string, pkgtypes.Pair[int, *types.AuthWebhookResponse]]( conf.AuthWebhookCacheSize, ) - authWebhookClient := webhook.NewClient[types.AuthWebhookRequest, types.AuthWebhookResponse]( - webhook.Options{ + authWebhookClient := pkgwebhook.NewClient[types.AuthWebhookRequest, types.AuthWebhookResponse]( + pkgwebhook.Options{ MaxRetries: conf.AuthWebhookMaxRetries, MinWaitInterval: conf.ParseAuthWebhookMinWaitInterval(), MaxWaitInterval: conf.ParseAuthWebhookMaxWaitInterval(), @@ -106,14 +107,14 @@ func New( }, ) - eventWebhookClient := webhook.NewClient[types.EventWebhookRequest, int]( - webhook.Options{ + eventWebhookManger := webhook.NewManager(pkgwebhook.NewClient[types.EventWebhookRequest, int]( + pkgwebhook.Options{ MaxRetries: conf.EventWebhookMaxRetries, MinWaitInterval: conf.ParseEventWebhookMinWaitInterval(), MaxWaitInterval: conf.ParseEventWebhookMaxWaitInterval(), RequestTimeout: conf.ParseEventWebhookRequestTimeout(), }, - ) + )) // 03. Create pubsub, and locker. locker := sync.New() @@ -177,9 +178,9 @@ func New( return &Backend{ Config: conf, - AuthWebhookCache: authWebhookCache, - AuthWebhookClient: authWebhookClient, - EventWebhookClient: eventWebhookClient, + AuthWebhookCache: authWebhookCache, + AuthWebhookClient: authWebhookClient, + EventWebhookManager: eventWebhookManger, Locker: locker, PubSub: pubsub, diff --git a/server/backend/webhook/manager.go b/server/backend/webhook/manager.go new file mode 100644 index 000000000..69f476274 --- /dev/null +++ b/server/backend/webhook/manager.go @@ -0,0 +1,92 @@ +/* + * Copyright 2025 The Yorkie Authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package webhook provides publishing events to project endpoint. +package webhook + +import ( + "context" + "errors" + "fmt" + "net/http" + "time" + + "github.com/yorkie-team/yorkie/api/types" + "github.com/yorkie-team/yorkie/pkg/limit" + "github.com/yorkie-team/yorkie/pkg/webhook" +) + +const ( + expireInterval = 100 * time.Millisecond + webhookWindow = 5 * time.Second + tokenTTL = 10 * time.Second +) + +var ( + // ErrUnexpectedStatusCode is returned when the webhook returns an unexpected status code. + ErrUnexpectedStatusCode = errors.New("unexpected status code from webhook") +) + +// Manager manages sending webhook events with rate limiting. +type Manager struct { + limiter *limit.Limiter[types.EventRefKey] + webhookClient *webhook.Client[types.EventWebhookRequest, int] +} + +// NewManager creates a new instance of Manager with the provided webhook client. +func NewManager(cli *webhook.Client[types.EventWebhookRequest, int]) *Manager { + return &Manager{ + limiter: limit.New[types.EventRefKey](expireInterval, webhookWindow, tokenTTL), + webhookClient: cli, + } +} + +// Send dispatches a webhook event for the specified document and event reference key. +// It uses rate limiting to debounce multiple events within a short period. +func (m *Manager) Send(ctx context.Context, info types.EventWebhookInfo) error { + callback := func() error { + return sendWebhook(ctx, m.webhookClient, info.EventRefKey.EventWebhookType, info.Attribute) + } + + // If allowed immediately, invoke the callback. + if allowed := m.limiter.Allow(info.EventRefKey, callback); allowed { + return callback() + } + return nil +} + +// sendWebhook sends the webhook event using the provided client. +// It builds the request body and checks for a successful HTTP response. +func sendWebhook( + ctx context.Context, + cli *webhook.Client[types.EventWebhookRequest, int], + event types.EventWebhookType, + attr types.WebhookAttribute, +) error { + body, err := types.NewRequestBody(attr.DocKey, event) + if err != nil { + return fmt.Errorf("create webhook request body: %w", err) + } + + _, status, err := cli.Send(ctx, attr.URL, attr.SigningKey, body) + if err != nil { + return fmt.Errorf("send webhook event: %w", err) + } + if status != http.StatusOK { + return fmt.Errorf("webhook returned status %d: %w", status, ErrUnexpectedStatusCode) + } + return nil +} diff --git a/server/packs/packs.go b/server/packs/packs.go index c891fc9e6..766de6eab 100644 --- a/server/packs/packs.go +++ b/server/packs/packs.go @@ -37,7 +37,6 @@ import ( "github.com/yorkie-team/yorkie/server/backend/database" "github.com/yorkie-team/yorkie/server/backend/sync" "github.com/yorkie-team/yorkie/server/logging" - "github.com/yorkie-team/yorkie/server/webhook" ) // PushPullKey creates a new sync.Key of PushPull for the given document. @@ -200,13 +199,18 @@ func PushPull( ) if reqPack.OperationsLen() > 0 { - if err := webhook.SendEvent( - ctx, - be, - project, - docInfo.Key.String(), - events.DocRootChangedEvent, - ); err != nil { + info := types.EventWebhookInfo{ + EventRefKey: types.EventRefKey{ + DocRefKey: docRefKey, + EventWebhookType: events.DocRootChangedEvent.WebhookType(), + }, + Attribute: types.WebhookAttribute{ + SigningKey: project.SecretKey, + URL: project.EventWebhookURL, + DocKey: docInfo.Key.String(), + }, + } + if err := be.EventWebhookManager.Send(ctx, info); err != nil { logging.From(ctx).Error(err) return } diff --git a/server/webhook/events.go b/server/webhook/events.go deleted file mode 100644 index 76c007373..000000000 --- a/server/webhook/events.go +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Copyright 2025 The Yorkie Authors. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// Package webhook provides publishing events to project endpoint. -package webhook - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "net/http" - gotime "time" - - "github.com/yorkie-team/yorkie/api/types" - "github.com/yorkie-team/yorkie/api/types/events" - "github.com/yorkie-team/yorkie/server/backend" -) - -var ( - // ErrUnexpectedStatusCode is returned when the webhook returns an unexpected status code. - ErrUnexpectedStatusCode = errors.New("unexpected status code from webhook") -) - -// SendEvent sends an event to the project's event webhook endpoint. -func SendEvent( - ctx context.Context, - be *backend.Backend, - prj *types.Project, - docKey string, - eventType events.DocEventType, -) error { - webhookType := eventType.WebhookType() - if webhookType == "" { - return fmt.Errorf("invalid event webhook type: %s", eventType) - } - - if !prj.RequireEventWebhook(webhookType) { - return nil - } - - body, err := buildRequestBody(docKey, webhookType) - if err != nil { - return fmt.Errorf("marshal event webhook request: %w", err) - } - - _, status, err := be.EventWebhookClient.Send( - ctx, - prj.EventWebhookURL, - prj.SecretKey, - body, - ) - if err != nil { - return fmt.Errorf("send event webhook: %w", err) - } - if status != http.StatusOK { - return fmt.Errorf("send event webhook %d: %w", status, ErrUnexpectedStatusCode) - } - - return nil -} - -// buildRequestBody builds the request body for the event webhook. -func buildRequestBody(docKey string, webhookType types.EventWebhookType) ([]byte, error) { - req := types.EventWebhookRequest{ - Type: webhookType, - Attributes: types.EventWebhookAttribute{ - Key: docKey, - IssuedAt: gotime.Now().UTC().Format("2006-01-02T15:04:05.000Z"), - }, - } - - body, err := json.Marshal(req) - if err != nil { - return nil, fmt.Errorf("marshal event webhook request: %w", err) - } - - return body, nil -} From d9be94d954bfe219525ec69dbede04489331c5da Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Thu, 6 Mar 2025 17:17:33 +0900 Subject: [PATCH 16/37] Lint --- api/types/event_webhook.go | 9 ++++++--- api/types/resource_ref_key.go | 4 +++- pkg/limit/bucket.go | 10 ++++++---- server/backend/backend.go | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/api/types/event_webhook.go b/api/types/event_webhook.go index 9a1c6f394..a36712f8b 100644 --- a/api/types/event_webhook.go +++ b/api/types/event_webhook.go @@ -22,6 +22,7 @@ import ( "time" ) +// DateFormat defines the standard format used for timestamps. const DateFormat = "2006-01-02T15:04:05.000Z" // EventWebhookType represents event webhook type @@ -37,19 +38,19 @@ func IsValidEventType(eventType string) bool { return eventType == string(DocRootChanged) } -// EventWebhookAttribute represents the attribute of the webhook. +// EventWebhookAttribute represents metadata associated with a webhook event. type EventWebhookAttribute struct { Key string `json:"key"` IssuedAt string `json:"issuedAt"` } -// EventWebhookRequest represents the request of the webhook. +// EventWebhookRequest represents a webhook event request payload. type EventWebhookRequest struct { Type EventWebhookType `json:"type"` Attributes EventWebhookAttribute `json:"attributes"` } -// NewRequestBody builds the request body for the event webhook. +// NewRequestBody builds the JSON request body for a webhook event. func NewRequestBody(docKey string, event EventWebhookType) ([]byte, error) { req := EventWebhookRequest{ Type: event, @@ -66,11 +67,13 @@ func NewRequestBody(docKey string, event EventWebhookType) ([]byte, error) { return body, nil } +// EventWebhookInfo holds the webhook EventRefKey and its associated Attribute. type EventWebhookInfo struct { EventRefKey EventRefKey Attribute WebhookAttribute } +// WebhookAttribute defines attributes necessary for webhook handling. type WebhookAttribute struct { SigningKey string URL string diff --git a/api/types/resource_ref_key.go b/api/types/resource_ref_key.go index 1c036e0f6..0cafbee2c 100644 --- a/api/types/resource_ref_key.go +++ b/api/types/resource_ref_key.go @@ -53,11 +53,13 @@ func (r SnapshotRefKey) String() string { return fmt.Sprintf("Snapshot (%s.%s.%d)", r.ProjectID, r.DocID, r.ServerSeq) } +// EventRefKey represents an identifier used to reference an event. type EventRefKey struct { DocRefKey EventWebhookType } +// String returns the string representation of the given EventRefKey. func (r EventRefKey) String() string { - return fmt.Sprintf("DocEvent (%s.%s.%d)", r.ProjectID, r.DocID, r.EventWebhookType) + return fmt.Sprintf("DocEvent (%s.%s.%s)", r.ProjectID, r.DocID, r.EventWebhookType) } diff --git a/pkg/limit/bucket.go b/pkg/limit/bucket.go index d663c3cdf..52e113771 100644 --- a/pkg/limit/bucket.go +++ b/pkg/limit/bucket.go @@ -2,13 +2,13 @@ package limit import "time" -// Bucket is one token bucket that charged every window +// Bucket represents a single-token bucket that refills every specified time window. type Bucket struct { - window time.Duration - // last is the last time the limiter's tokens field was updated - last time.Time + window time.Duration // The interval at which the bucket refills. + last time.Time // The last time a token was granted. } +// NewBucket creates a new Bucket with the given initial time and refill window. func NewBucket(now time.Time, window time.Duration) Bucket { return Bucket{ window: window, @@ -16,6 +16,8 @@ func NewBucket(now time.Time, window time.Duration) Bucket { } } +// Allow checks if a token can be granted at the given time. +// It returns true if the time has advanced past the refill window, otherwise false. func (b *Bucket) Allow(now time.Time) bool { if now.Before(b.last.Add(b.window)) { return false diff --git a/server/backend/backend.go b/server/backend/backend.go index 9cfb64103..42d6c4303 100644 --- a/server/backend/backend.go +++ b/server/backend/backend.go @@ -22,7 +22,6 @@ package backend import ( "context" "fmt" - "github.com/yorkie-team/yorkie/server/backend/webhook" "os" "github.com/yorkie-team/yorkie/api/types" @@ -37,6 +36,7 @@ import ( "github.com/yorkie-team/yorkie/server/backend/messagebroker" "github.com/yorkie-team/yorkie/server/backend/pubsub" "github.com/yorkie-team/yorkie/server/backend/sync" + "github.com/yorkie-team/yorkie/server/backend/webhook" "github.com/yorkie-team/yorkie/server/logging" "github.com/yorkie-team/yorkie/server/profiling/prometheus" ) From 02ea2260f034c28085adfc518f10a9527875c1b5 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Thu, 6 Mar 2025 18:41:26 +0900 Subject: [PATCH 17/37] Fix error handling --- pkg/limit/limiter.go | 9 ++++----- server/backend/webhook/manager.go | 9 ++++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index f374a81dd..027075b8b 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -65,13 +65,13 @@ type limiterEntry[K comparable] struct { key K bucket Bucket expireTime time.Time - debouncingCallback func() error + debouncingCallback func() } // Allow checks if an event is allowed for the given key based on the rate bucket. // If allowed, it clears any pending debouncing callback; otherwise, it stores the provided callback. // It returns true if the event is allowed immediately. -func (l *Limiter[K]) Allow(key K, callback func() error) bool { +func (l *Limiter[K]) Allow(key K, callback func()) bool { l.mu.Lock() defer l.mu.Unlock() @@ -110,7 +110,7 @@ func (l *Limiter[K]) expirationLoop() { for { select { case <-ticker.C: - l.expireEntries() + go l.expireEntries() case <-l.closeChan: return } @@ -144,8 +144,7 @@ func (l *Limiter[K]) expireEntries() { // Process debouncing callbacks for expired entries. for _, entry := range expiredEntries { if entry.debouncingCallback != nil { - if err := entry.debouncingCallback(); err != nil { - } + entry.debouncingCallback() } } } diff --git a/server/backend/webhook/manager.go b/server/backend/webhook/manager.go index 69f476274..4afd5c535 100644 --- a/server/backend/webhook/manager.go +++ b/server/backend/webhook/manager.go @@ -27,6 +27,7 @@ import ( "github.com/yorkie-team/yorkie/api/types" "github.com/yorkie-team/yorkie/pkg/limit" "github.com/yorkie-team/yorkie/pkg/webhook" + "github.com/yorkie-team/yorkie/server/logging" ) const ( @@ -57,13 +58,15 @@ func NewManager(cli *webhook.Client[types.EventWebhookRequest, int]) *Manager { // Send dispatches a webhook event for the specified document and event reference key. // It uses rate limiting to debounce multiple events within a short period. func (m *Manager) Send(ctx context.Context, info types.EventWebhookInfo) error { - callback := func() error { - return sendWebhook(ctx, m.webhookClient, info.EventRefKey.EventWebhookType, info.Attribute) + callback := func() { + if err := sendWebhook(ctx, m.webhookClient, info.EventRefKey.EventWebhookType, info.Attribute); err != nil { + logging.From(ctx).Error(err) + } } // If allowed immediately, invoke the callback. if allowed := m.limiter.Allow(info.EventRefKey, callback); allowed { - return callback() + return sendWebhook(ctx, m.webhookClient, info.EventRefKey.EventWebhookType, info.Attribute) } return nil } From 1222c0904c5bb87805f9ccc43fee50a560a6c18d Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Fri, 7 Mar 2025 10:33:58 +0900 Subject: [PATCH 18/37] Rename constant --- pkg/limit/limiter.go | 22 +++++++++++----------- server/backend/webhook/manager.go | 6 +++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index 027075b8b..173de7690 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -30,8 +30,8 @@ type Limiter[K comparable] struct { closeChan chan struct{} expireInterval time.Duration - rateWindow time.Duration - entryTTL time.Duration + throttleWindow time.Duration + debouncingTime time.Duration // evictionList holds the limiter entries in order of recency. evictionList *list.List @@ -43,14 +43,14 @@ type Limiter[K comparable] struct { // Parameters: // // expireInterval: How often to check for expired entries. -// rateWindow: The time window for rate limiting. -// entryTTL: The time-to-live for each rate bucket entry. -func New[K comparable](expireInterval, rateWindow, entryTTL time.Duration) *Limiter[K] { +// throttleWindow: The time window for rate limiting. +// debouncingTime: The time-to-live for each rate bucket entry. +func New[K comparable](expire, throttle, debouncing time.Duration) *Limiter[K] { lim := &Limiter[K]{ closeChan: make(chan struct{}), - expireInterval: expireInterval, - rateWindow: rateWindow, - entryTTL: entryTTL, + expireInterval: expire, + throttleWindow: throttle, + debouncingTime: debouncing, evictionList: list.New(), entries: make(map[K]*list.Element), } @@ -86,16 +86,16 @@ func (l *Limiter[K]) Allow(key K, callback func()) bool { } // Update recency and extend TTL. l.evictionList.MoveToFront(elem) - entry.expireTime = now.Add(l.entryTTL) + entry.expireTime = now.Add(l.throttleWindow + l.debouncingTime) return allowed } // Create a new rate bucket for a new key. - bucket := NewBucket(now, l.rateWindow) + bucket := NewBucket(now, l.throttleWindow) entry := &limiterEntry[K]{ key: key, bucket: bucket, - expireTime: now.Add(l.entryTTL), + expireTime: now.Add(l.throttleWindow + l.debouncingTime), } elem := l.evictionList.PushFront(entry) l.entries[key] = elem diff --git a/server/backend/webhook/manager.go b/server/backend/webhook/manager.go index 4afd5c535..dc813bb70 100644 --- a/server/backend/webhook/manager.go +++ b/server/backend/webhook/manager.go @@ -32,8 +32,8 @@ import ( const ( expireInterval = 100 * time.Millisecond - webhookWindow = 5 * time.Second - tokenTTL = 10 * time.Second + throttleWindow = 5 * time.Second + debouncingTime = 3 * time.Second ) var ( @@ -50,7 +50,7 @@ type Manager struct { // NewManager creates a new instance of Manager with the provided webhook client. func NewManager(cli *webhook.Client[types.EventWebhookRequest, int]) *Manager { return &Manager{ - limiter: limit.New[types.EventRefKey](expireInterval, webhookWindow, tokenTTL), + limiter: limit.New[types.EventRefKey](expireInterval, throttleWindow, debouncingTime), webhookClient: cli, } } From 5bdad0cfb444e04bca971d898d1b0c933fb73689 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Fri, 7 Mar 2025 10:50:21 +0900 Subject: [PATCH 19/37] Execute remain job when closing --- pkg/limit/limiter.go | 43 +++++++--- pkg/limit/limiter_test.go | 167 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+), 13 deletions(-) create mode 100644 pkg/limit/limiter_test.go diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index 173de7690..9b40537ac 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -27,11 +27,13 @@ import ( // It maintains a single token bucket. type Limiter[K comparable] struct { mu sync.Mutex + wg sync.WaitGroup closeChan chan struct{} expireInterval time.Duration throttleWindow time.Duration debouncingTime time.Duration + expireBatch int // evictionList holds the limiter entries in order of recency. evictionList *list.List @@ -45,12 +47,13 @@ type Limiter[K comparable] struct { // expireInterval: How often to check for expired entries. // throttleWindow: The time window for rate limiting. // debouncingTime: The time-to-live for each rate bucket entry. -func New[K comparable](expire, throttle, debouncing time.Duration) *Limiter[K] { +func New[K comparable](expireNum int, expire, throttle, debouncing time.Duration) *Limiter[K] { lim := &Limiter[K]{ closeChan: make(chan struct{}), expireInterval: expire, throttleWindow: throttle, debouncingTime: debouncing, + expireBatch: expireNum, evictionList: list.New(), entries: make(map[K]*list.Element), } @@ -110,21 +113,23 @@ func (l *Limiter[K]) expirationLoop() { for { select { case <-ticker.C: - go l.expireEntries() + expiredEntries := l.collectExpired() + l.runDebounce(expiredEntries) case <-l.closeChan: return } } } -// expireEntries checks for and removes expired entries from the limiter. -// It also triggers any stored debouncing callbacks for expired entries. -func (l *Limiter[K]) expireEntries() { +// collectExpired gathers expired entries and removes them from the limiter. +func (l *Limiter[K]) collectExpired() []*limiterEntry[K] { now := time.Now() - expiredEntries := make([]*limiterEntry[K], 0, 100) + expiredEntries := make([]*limiterEntry[K], 0, l.expireBatch) l.mu.Lock() - for { + defer l.mu.Unlock() + + for range l.expireBatch { elem := l.evictionList.Back() if elem == nil { break @@ -139,17 +144,29 @@ func (l *Limiter[K]) expireEntries() { l.evictionList.Remove(elem) delete(l.entries, entry.key) } - l.mu.Unlock() - // Process debouncing callbacks for expired entries. - for _, entry := range expiredEntries { - if entry.debouncingCallback != nil { - entry.debouncingCallback() + return expiredEntries +} + +// runDebounce runs the debouncing callbacks for expired entries asynchronously. +func (l *Limiter[K]) runDebounce(entries []*limiterEntry[K]) { + l.wg.Add(1) + go func() { + defer l.wg.Done() + for _, entry := range entries { + if entry.debouncingCallback != nil { + entry.debouncingCallback() + } } - } + }() } // Close terminates the expiration loop and cleans up resources. func (l *Limiter[K]) Close() { close(l.closeChan) + for expiredEntries := l.collectExpired(); len(expiredEntries) > 0; { + l.runDebounce(expiredEntries) + } + + l.wg.Wait() } diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go new file mode 100644 index 000000000..9d20f556a --- /dev/null +++ b/pkg/limit/limiter_test.go @@ -0,0 +1,167 @@ +/* + * Copyright 2025 The Yorkie Authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package limit_test provides unit tests for event timing control components. +package limit_test + +import ( + "context" + "github.com/yorkie-team/yorkie/pkg/limit" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +// TestSynchronousExecute verifies the behavior of synchronous calls to the throttler. +func TestSynchronousExecute(t *testing.T) { + const ( + expireInterval = 10 * time.Millisecond + throttleWindow = 100 * time.Millisecond + debouncingTime = 100 * time.Millisecond + waitingTime = expireInterval + throttleWindow + debouncingTime + ) + + t.Run("Single call test", func(t *testing.T) { + lim := limit.New[string](expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + if lim.Allow("key", callback) { + callback() + } + time.Sleep(waitingTime) + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + }) + + t.Run("Many synchronous call with trailing debounced event", func(t *testing.T) { + const numExecute = 100000 + lim := limit.New[string](expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + for range numExecute { + if lim.Allow("key", callback) { + callback() + } + } + time.Sleep(waitingTime) + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + }) +} + +// TestConcurrentExecute verifies the throttler behavior under concurrent execution scenarios. +func TestConcurrentExecute(t *testing.T) { + const ( + expireInterval = 10 * time.Millisecond + throttleWindow = 100 * time.Millisecond + debouncingTime = 100 * time.Millisecond + waitingTime = expireInterval + throttleWindow + debouncingTime + ) + + t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { + const numRoutines = 100000 + lim := limit.New[string](expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + for range numRoutines { + go func() { + if lim.Allow("key", callback) { + callback() + } + }() + } + + time.Sleep(waitingTime) + // Expect one immediate and one trailing callback execution. + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + }) + + t.Run("Concurrent calls with different keys", func(t *testing.T) { + const numRoutines = 100000 + lim := limit.New[int](expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + for i := range numRoutines { + i := i + go func() { + if lim.Allow(i, callback) { + callback() + } + }() + } + + time.Sleep(waitingTime) + // Expect exactly one immediate and one trailing callback execution. + assert.Equal(t, int32(numRoutines), atomic.LoadInt32(&callCount)) + }) + + // This test simulates a continuous stream of events. + // It triggers multiple concurrent calls at a regular interval and checks that throttling + // limits the total number of callback invocations to one per window plus one trailing call. + t.Run("Throttling over continuous event stream", func(t *testing.T) { + const ( + numWindows = 10 // Number of throttle windows. + eventPerWindow = 1000 // Number of events triggered within each window. + numRoutines = 100 // Number of concurrent calls per tick. + totalDuration = throttleWindow * time.Duration(numWindows) // Total simulation duration. + eventInterval = throttleWindow / time.Duration(eventPerWindow) // Interval between events. + ) + + lim := limit.New[string](expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + ticker := time.NewTicker(eventInterval) + defer ticker.Stop() + timeCtx, cancel := context.WithTimeout(context.Background(), totalDuration) + defer cancel() + + // Continuously trigger events until the simulation ends. + for { + select { + case <-ticker.C: + // Each tick triggers multiple concurrent calls. + for range numRoutines { + go func() { + if lim.Allow("key", callback) { + callback() + } + }() + } + case <-timeCtx.Done(): + // After the event stream stops, allow time for any trailing callback to execute. + time.Sleep(waitingTime) + // Expect one callback per throttle window plus one additional trailing call. + assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) + return + } + } + }) +} From 982bcf5797d815404bd1e4f6c73d2354694de08c Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Fri, 7 Mar 2025 12:21:49 +0900 Subject: [PATCH 20/37] Add expire batch configuration option --- pkg/limit/limiter.go | 8 ++-- pkg/limit/limiter_test.go | 79 ++++++++++++++++++++++++++----- server/backend/webhook/manager.go | 3 +- 3 files changed, 72 insertions(+), 18 deletions(-) diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index 9b40537ac..bd2946c35 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -113,7 +113,7 @@ func (l *Limiter[K]) expirationLoop() { for { select { case <-ticker.C: - expiredEntries := l.collectExpired() + expiredEntries := l.collectExpired(l.expireBatch) l.runDebounce(expiredEntries) case <-l.closeChan: return @@ -122,9 +122,9 @@ func (l *Limiter[K]) expirationLoop() { } // collectExpired gathers expired entries and removes them from the limiter. -func (l *Limiter[K]) collectExpired() []*limiterEntry[K] { +func (l *Limiter[K]) collectExpired(expireBatch int) []*limiterEntry[K] { now := time.Now() - expiredEntries := make([]*limiterEntry[K], 0, l.expireBatch) + expiredEntries := make([]*limiterEntry[K], 0, expireBatch) l.mu.Lock() defer l.mu.Unlock() @@ -164,7 +164,7 @@ func (l *Limiter[K]) runDebounce(entries []*limiterEntry[K]) { // Close terminates the expiration loop and cleans up resources. func (l *Limiter[K]) Close() { close(l.closeChan) - for expiredEntries := l.collectExpired(); len(expiredEntries) > 0; { + for expiredEntries := l.collectExpired(l.expireBatch / 10); len(expiredEntries) > 0; { l.runDebounce(expiredEntries) } diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index 9d20f556a..32e823322 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -19,25 +19,28 @@ package limit_test import ( "context" - "github.com/yorkie-team/yorkie/pkg/limit" + "fmt" "sync/atomic" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/yorkie-team/yorkie/pkg/limit" ) // TestSynchronousExecute verifies the behavior of synchronous calls to the throttler. func TestSynchronousExecute(t *testing.T) { const ( + expireBatch = 100 expireInterval = 10 * time.Millisecond throttleWindow = 100 * time.Millisecond debouncingTime = 100 * time.Millisecond waitingTime = expireInterval + throttleWindow + debouncingTime + numExecute = 10000 ) t.Run("Single call test", func(t *testing.T) { - lim := limit.New[string](expireInterval, throttleWindow, debouncingTime) + lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) @@ -48,11 +51,12 @@ func TestSynchronousExecute(t *testing.T) { } time.Sleep(waitingTime) assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + lim.Close() + assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) }) t.Run("Many synchronous call with trailing debounced event", func(t *testing.T) { - const numExecute = 100000 - lim := limit.New[string](expireInterval, throttleWindow, debouncingTime) + lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) @@ -63,29 +67,34 @@ func TestSynchronousExecute(t *testing.T) { callback() } } + time.Sleep(waitingTime) assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + lim.Close() + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) }) } // TestConcurrentExecute verifies the throttler behavior under concurrent execution scenarios. func TestConcurrentExecute(t *testing.T) { const ( + expireBatch = 10000 expireInterval = 10 * time.Millisecond throttleWindow = 100 * time.Millisecond debouncingTime = 100 * time.Millisecond waitingTime = expireInterval + throttleWindow + debouncingTime + numExecute = 10000 ) t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { - const numRoutines = 100000 - lim := limit.New[string](expireInterval, throttleWindow, debouncingTime) + lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) + var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) } - for range numRoutines { + for range numExecute { go func() { if lim.Allow("key", callback) { callback() @@ -96,17 +105,18 @@ func TestConcurrentExecute(t *testing.T) { time.Sleep(waitingTime) // Expect one immediate and one trailing callback execution. assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + lim.Close() + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) }) t.Run("Concurrent calls with different keys", func(t *testing.T) { - const numRoutines = 100000 - lim := limit.New[int](expireInterval, throttleWindow, debouncingTime) + lim := limit.New[int](expireBatch, expireInterval, throttleWindow, debouncingTime) var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) } - for i := range numRoutines { + for i := range numExecute { i := i go func() { if lim.Allow(i, callback) { @@ -116,8 +126,10 @@ func TestConcurrentExecute(t *testing.T) { } time.Sleep(waitingTime) - // Expect exactly one immediate and one trailing callback execution. - assert.Equal(t, int32(numRoutines), atomic.LoadInt32(&callCount)) + // For different keys, every call is immediate so expect numExecute callbacks. + assert.Equal(t, int32(numExecute), atomic.LoadInt32(&callCount)) + lim.Close() + assert.Equal(t, int32(numExecute), atomic.LoadInt32(&callCount)) }) // This test simulates a continuous stream of events. @@ -132,7 +144,8 @@ func TestConcurrentExecute(t *testing.T) { eventInterval = throttleWindow / time.Duration(eventPerWindow) // Interval between events. ) - lim := limit.New[string](expireInterval, throttleWindow, debouncingTime) + lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) + var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) @@ -160,8 +173,48 @@ func TestConcurrentExecute(t *testing.T) { time.Sleep(waitingTime) // Expect one callback per throttle window plus one additional trailing call. assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) + lim.Close() + assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) return } } }) } + +// TestExpireBatch verifies that the expiration loop processes expired entries in batches. +func TestExpireBatch(t *testing.T) { + const ( + expireBatch = 10 + expireInterval = 10 * time.Millisecond + throttleWindow = 50 * time.Millisecond + debouncingTime = 50 * time.Millisecond + waitingTime = expireInterval + throttleWindow + debouncingTime + totalKeys = expireBatch * 3 // create more keys than one batch + ) + + lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + // Create totalKeys entries, each with one immediate call and one trailing (debounced) callback. + // For each key, the first call is allowed (and calls callback immediately), + // while the second call queues the callback. + for i := 0; i < totalKeys; i++ { + key := fmt.Sprintf("key-%d", i) + // First call: immediate execution. + if lim.Allow(key, callback) { + callback() + } + // Second call: queues the debounced callback. + lim.Allow(key, callback) + } + + // Wait for all debounced callbacks to run. The expirationLoop will process them in batches. + time.Sleep(waitingTime) + + // For each key, we expect one immediate call and one debounced call. + expected := int32(totalKeys * 2) + assert.Equal(t, expected, atomic.LoadInt32(&callCount)) +} diff --git a/server/backend/webhook/manager.go b/server/backend/webhook/manager.go index dc813bb70..e1ad6c818 100644 --- a/server/backend/webhook/manager.go +++ b/server/backend/webhook/manager.go @@ -34,6 +34,7 @@ const ( expireInterval = 100 * time.Millisecond throttleWindow = 5 * time.Second debouncingTime = 3 * time.Second + expireBatch = 100 ) var ( @@ -50,7 +51,7 @@ type Manager struct { // NewManager creates a new instance of Manager with the provided webhook client. func NewManager(cli *webhook.Client[types.EventWebhookRequest, int]) *Manager { return &Manager{ - limiter: limit.New[types.EventRefKey](expireInterval, throttleWindow, debouncingTime), + limiter: limit.New[types.EventRefKey](expireBatch, expireInterval, throttleWindow, debouncingTime), webhookClient: cli, } } From b16b7ef509092824aaf8fdc14ef4356d78b77cbd Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Fri, 7 Mar 2025 14:15:16 +0900 Subject: [PATCH 21/37] Add NewEventWebhookInfo --- api/types/event_webhook.go | 19 +++++++++++++++++++ server/packs/packs.go | 18 +++++++----------- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/api/types/event_webhook.go b/api/types/event_webhook.go index a36712f8b..6ea0d991b 100644 --- a/api/types/event_webhook.go +++ b/api/types/event_webhook.go @@ -73,6 +73,25 @@ type EventWebhookInfo struct { Attribute WebhookAttribute } +// NewEventWebhookInfo initializes an EventWebhookInfo with the given parameters. +func NewEventWebhookInfo( + docRefKey DocRefKey, + event EventWebhookType, + signingKey, url, docKey string, +) EventWebhookInfo { + return EventWebhookInfo{ + EventRefKey: EventRefKey{ + DocRefKey: docRefKey, + EventWebhookType: event, + }, + Attribute: WebhookAttribute{ + SigningKey: signingKey, + URL: url, + DocKey: docKey, + }, + } +} + // WebhookAttribute defines attributes necessary for webhook handling. type WebhookAttribute struct { SigningKey string diff --git a/server/packs/packs.go b/server/packs/packs.go index 766de6eab..5d6601461 100644 --- a/server/packs/packs.go +++ b/server/packs/packs.go @@ -199,17 +199,13 @@ func PushPull( ) if reqPack.OperationsLen() > 0 { - info := types.EventWebhookInfo{ - EventRefKey: types.EventRefKey{ - DocRefKey: docRefKey, - EventWebhookType: events.DocRootChangedEvent.WebhookType(), - }, - Attribute: types.WebhookAttribute{ - SigningKey: project.SecretKey, - URL: project.EventWebhookURL, - DocKey: docInfo.Key.String(), - }, - } + info := types.NewEventWebhookInfo( + docRefKey, + events.DocRootChangedEvent.WebhookType(), + project.SecretKey, + project.EventWebhookURL, + docInfo.Key.String(), + ) if err := be.EventWebhookManager.Send(ctx, info); err != nil { logging.From(ctx).Error(err) return From 5dcab93409b9be82a2d3186d316a93f9267567e9 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Fri, 7 Mar 2025 15:08:41 +0900 Subject: [PATCH 22/37] Add close event webhook manager --- pkg/limit/limiter.go | 60 ++++++------- pkg/limit/limiter_test.go | 139 ++++++++++++++++++------------ server/backend/backend.go | 2 + server/backend/webhook/manager.go | 15 ++-- 4 files changed, 125 insertions(+), 91 deletions(-) diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index bd2946c35..33b4f5415 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -26,14 +26,14 @@ import ( // Limiter provides rate limiting functionality with a debouncing callback. // It maintains a single token bucket. type Limiter[K comparable] struct { - mu sync.Mutex - wg sync.WaitGroup - closeChan chan struct{} + mu sync.Mutex + wg sync.WaitGroup + closing chan struct{} - expireInterval time.Duration - throttleWindow time.Duration - debouncingTime time.Duration - expireBatch int + expireInterval time.Duration + throttleWindow time.Duration + debouncingTime time.Duration + expireBatchSize int // evictionList holds the limiter entries in order of recency. evictionList *list.List @@ -41,21 +41,21 @@ type Limiter[K comparable] struct { entries map[K]*list.Element } -// New creates and returns a new Limiter instance. +// NewLimiter creates and returns a new Limiter instance. // Parameters: // // expireInterval: How often to check for expired entries. // throttleWindow: The time window for rate limiting. // debouncingTime: The time-to-live for each rate bucket entry. -func New[K comparable](expireNum int, expire, throttle, debouncing time.Duration) *Limiter[K] { +func NewLimiter[K comparable](expireNum int, expire, throttle, debouncing time.Duration) *Limiter[K] { lim := &Limiter[K]{ - closeChan: make(chan struct{}), - expireInterval: expire, - throttleWindow: throttle, - debouncingTime: debouncing, - expireBatch: expireNum, - evictionList: list.New(), - entries: make(map[K]*list.Element), + closing: make(chan struct{}), + expireInterval: expire, + throttleWindow: throttle, + debouncingTime: debouncing, + expireBatchSize: expireNum, + evictionList: list.New(), + entries: make(map[K]*list.Element), } // Start the background expiration process. @@ -113,34 +113,36 @@ func (l *Limiter[K]) expirationLoop() { for { select { case <-ticker.C: - expiredEntries := l.collectExpired(l.expireBatch) + expiredEntries := l.collectEntries(true) l.runDebounce(expiredEntries) - case <-l.closeChan: + case <-l.closing: return } } } -// collectExpired gathers expired entries and removes them from the limiter. -func (l *Limiter[K]) collectExpired(expireBatch int) []*limiterEntry[K] { +// collectEntries gathers expired entries and removes them from the limiter. +func (l *Limiter[K]) collectEntries(onlyExpired bool) []*limiterEntry[K] { now := time.Now() - expiredEntries := make([]*limiterEntry[K], 0, expireBatch) + expiredEntries := make([]*limiterEntry[K], 0, l.expireBatchSize) l.mu.Lock() defer l.mu.Unlock() - for range l.expireBatch { + for range l.expireBatchSize { elem := l.evictionList.Back() if elem == nil { break } entry := elem.Value.(*limiterEntry[K]) - if now.Before(entry.expireTime) { + if onlyExpired && now.Before(entry.expireTime) { break } - expiredEntries = append(expiredEntries, entry) + if entry.debouncingCallback != nil { + expiredEntries = append(expiredEntries, entry) + } l.evictionList.Remove(elem) delete(l.entries, entry.key) } @@ -154,17 +156,17 @@ func (l *Limiter[K]) runDebounce(entries []*limiterEntry[K]) { go func() { defer l.wg.Done() for _, entry := range entries { - if entry.debouncingCallback != nil { - entry.debouncingCallback() - } + entry.debouncingCallback() } }() } // Close terminates the expiration loop and cleans up resources. func (l *Limiter[K]) Close() { - close(l.closeChan) - for expiredEntries := l.collectExpired(l.expireBatch / 10); len(expiredEntries) > 0; { + close(l.closing) + + for l.evictionList.Len() > 0 { + expiredEntries := l.collectEntries(false) l.runDebounce(expiredEntries) } diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index 32e823322..ee510feda 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -28,19 +28,19 @@ import ( "github.com/yorkie-team/yorkie/pkg/limit" ) -// TestSynchronousExecute verifies the behavior of synchronous calls to the throttler. -func TestSynchronousExecute(t *testing.T) { +// TestSynchronousExecution verifies the behavior of synchronous calls to the throttler. +func TestSynchronousExecution(t *testing.T) { const ( - expireBatch = 100 - expireInterval = 10 * time.Millisecond - throttleWindow = 100 * time.Millisecond - debouncingTime = 100 * time.Millisecond - waitingTime = expireInterval + throttleWindow + debouncingTime - numExecute = 10000 + expireBatchSize = 100 + expireInterval = 10 * time.Millisecond + throttleWindow = 100 * time.Millisecond + debouncingTime = 100 * time.Millisecond + waitingTime = expireInterval + throttleWindow + debouncingTime + numExecute = 10000 ) - t.Run("Single call test", func(t *testing.T) { - lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) + t.Run("Single Call", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) @@ -55,8 +55,8 @@ func TestSynchronousExecute(t *testing.T) { assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) }) - t.Run("Many synchronous call with trailing debounced event", func(t *testing.T) { - lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) + t.Run("Multiple Synchronous Calls with Trailing Debounce", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) @@ -75,19 +75,19 @@ func TestSynchronousExecute(t *testing.T) { }) } -// TestConcurrentExecute verifies the throttler behavior under concurrent execution scenarios. -func TestConcurrentExecute(t *testing.T) { +// TestConcurrentExecution verifies the throttler behavior under concurrent execution scenarios. +func TestConcurrentExecution(t *testing.T) { const ( - expireBatch = 10000 - expireInterval = 10 * time.Millisecond - throttleWindow = 100 * time.Millisecond - debouncingTime = 100 * time.Millisecond - waitingTime = expireInterval + throttleWindow + debouncingTime - numExecute = 10000 + expireBatchSize = 10000 + expireInterval = 10 * time.Millisecond + throttleWindow = 100 * time.Millisecond + debouncingTime = 100 * time.Millisecond + waitingTime = expireInterval + throttleWindow + debouncingTime + numExecute = 10000 ) - t.Run("Concurrent calls result in one immediate and one trailing execution", func(t *testing.T) { - lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) + t.Run("Concurrent Calls: Single Immediate and Trailing Execution", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) var callCount int32 callback := func() { @@ -109,8 +109,8 @@ func TestConcurrentExecute(t *testing.T) { assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) }) - t.Run("Concurrent calls with different keys", func(t *testing.T) { - lim := limit.New[int](expireBatch, expireInterval, throttleWindow, debouncingTime) + t.Run("Concurrent Calls with Different Keys", func(t *testing.T) { + lim := limit.NewLimiter[int](expireBatchSize, expireInterval, throttleWindow, debouncingTime) var callCount int32 callback := func() { atomic.AddInt32(&callCount, 1) @@ -132,10 +132,7 @@ func TestConcurrentExecute(t *testing.T) { assert.Equal(t, int32(numExecute), atomic.LoadInt32(&callCount)) }) - // This test simulates a continuous stream of events. - // It triggers multiple concurrent calls at a regular interval and checks that throttling - // limits the total number of callback invocations to one per window plus one trailing call. - t.Run("Throttling over continuous event stream", func(t *testing.T) { + t.Run("Continuous Event Stream Throttling", func(t *testing.T) { const ( numWindows = 10 // Number of throttle windows. eventPerWindow = 1000 // Number of events triggered within each window. @@ -144,7 +141,7 @@ func TestConcurrentExecute(t *testing.T) { eventInterval = throttleWindow / time.Duration(eventPerWindow) // Interval between events. ) - lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) var callCount int32 callback := func() { @@ -169,7 +166,7 @@ func TestConcurrentExecute(t *testing.T) { }() } case <-timeCtx.Done(): - // After the event stream stops, allow time for any trailing callback to execute. + // Allow time for any trailing callback to execute. time.Sleep(waitingTime) // Expect one callback per throttle window plus one additional trailing call. assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) @@ -181,40 +178,68 @@ func TestConcurrentExecute(t *testing.T) { }) } -// TestExpireBatch verifies that the expiration loop processes expired entries in batches. -func TestExpireBatch(t *testing.T) { +// TestBatchExpiration verifies that the expiration loop processes expired entries in batches. +func TestBatchExpiration(t *testing.T) { const ( - expireBatch = 10 expireInterval = 10 * time.Millisecond throttleWindow = 50 * time.Millisecond debouncingTime = 50 * time.Millisecond - waitingTime = expireInterval + throttleWindow + debouncingTime - totalKeys = expireBatch * 3 // create more keys than one batch + + expireBatchSize = 100 + batchNum int32 = 10 + + totalKeys = expireBatchSize * batchNum // create more keys than one batch ) - lim := limit.New[string](expireBatch, expireInterval, throttleWindow, debouncingTime) - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } - - // Create totalKeys entries, each with one immediate call and one trailing (debounced) callback. - // For each key, the first call is allowed (and calls callback immediately), - // while the second call queues the callback. - for i := 0; i < totalKeys; i++ { - key := fmt.Sprintf("key-%d", i) - // First call: immediate execution. - if lim.Allow(key, callback) { - callback() + t.Run("Process Expire Batch", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) } - // Second call: queues the debounced callback. - lim.Allow(key, callback) - } - // Wait for all debounced callbacks to run. The expirationLoop will process them in batches. - time.Sleep(waitingTime) + // For each key: first call executes immediately, second call schedules a debounced callback. + for i := range totalKeys { + key := fmt.Sprintf("key-%d", i) + // Immediate execution. + if lim.Allow(key, callback) { + callback() + } + // Queue the debounced callback. + lim.Allow(key, callback) + } - // For each key, we expect one immediate call and one debounced call. - expected := int32(totalKeys * 2) - assert.Equal(t, expected, atomic.LoadInt32(&callCount)) + assert.Equal(t, totalKeys, atomic.LoadInt32(&callCount)) + + time.Sleep(throttleWindow + debouncingTime + expireInterval/2) + for i := int32(1); i <= batchNum; i++ { + assert.Equal(t, totalKeys+expireBatchSize*i, atomic.LoadInt32(&callCount)) + time.Sleep(expireInterval) + } + assert.Equal(t, totalKeys+expireBatchSize*batchNum, atomic.LoadInt32(&callCount)) + lim.Close() + assert.Equal(t, totalKeys+expireBatchSize*batchNum, atomic.LoadInt32(&callCount)) + }) + + t.Run("Force Close Expired", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + for i := range totalKeys { + key := fmt.Sprintf("key-%d", i) + // Immediate execution. + if lim.Allow(key, callback) { + callback() + } + // Queue the debounced callback. + lim.Allow(key, callback) + } + + assert.Equal(t, totalKeys, atomic.LoadInt32(&callCount)) + lim.Close() + assert.Equal(t, totalKeys*2, atomic.LoadInt32(&callCount)) + }) } diff --git a/server/backend/backend.go b/server/backend/backend.go index 42d6c4303..e80f7d5e6 100644 --- a/server/backend/backend.go +++ b/server/backend/backend.go @@ -211,6 +211,8 @@ func (b *Backend) Shutdown() error { b.Background.Close() + b.EventWebhookManager.Close() + if err := b.MsgBroker.Close(); err != nil { logging.DefaultLogger().Error(err) } diff --git a/server/backend/webhook/manager.go b/server/backend/webhook/manager.go index e1ad6c818..03f3a4586 100644 --- a/server/backend/webhook/manager.go +++ b/server/backend/webhook/manager.go @@ -31,10 +31,10 @@ import ( ) const ( - expireInterval = 100 * time.Millisecond - throttleWindow = 5 * time.Second - debouncingTime = 3 * time.Second - expireBatch = 100 + expireInterval = 100 * time.Millisecond + throttleWindow = 5 * time.Second + debouncingTime = 3 * time.Second + expireBatchSize = 100 ) var ( @@ -51,7 +51,7 @@ type Manager struct { // NewManager creates a new instance of Manager with the provided webhook client. func NewManager(cli *webhook.Client[types.EventWebhookRequest, int]) *Manager { return &Manager{ - limiter: limit.New[types.EventRefKey](expireBatch, expireInterval, throttleWindow, debouncingTime), + limiter: limit.NewLimiter[types.EventRefKey](expireBatchSize, expireInterval, throttleWindow, debouncingTime), webhookClient: cli, } } @@ -72,6 +72,11 @@ func (m *Manager) Send(ctx context.Context, info types.EventWebhookInfo) error { return nil } +// Close closes the event webhook manager. This will wait for flushing remain debouncing events +func (m *Manager) Close() { + m.limiter.Close() +} + // sendWebhook sends the webhook event using the provided client. // It builds the request body and checks for a successful HTTP response. func sendWebhook( From 5ed551c24352b5ee74b03a25d789920c28669ea7 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Fri, 7 Mar 2025 15:21:10 +0900 Subject: [PATCH 23/37] Remove golang rate package --- go.mod | 5 +---- go.sum | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 496bde4d6..3f99000a4 100644 --- a/go.mod +++ b/go.mod @@ -32,10 +32,7 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -require ( - github.com/pierrec/lz4/v4 v4.1.15 // indirect - golang.org/x/time v0.10.0 // indirect -) +require github.com/pierrec/lz4/v4 v4.1.15 // indirect require ( github.com/beorn7/perks v1.0.1 // indirect diff --git a/go.sum b/go.sum index 14665358d..3bd98928d 100644 --- a/go.sum +++ b/go.sum @@ -575,8 +575,6 @@ golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= -golang.org/x/time v0.10.0 h1:3usCWA8tQn0L8+hFJQNgzpWbd89begxN66o1Ojdn5L4= -golang.org/x/time v0.10.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= From 9e93ec1743275e3a0c467e2c069972efeac89a6d Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Fri, 7 Mar 2025 15:21:48 +0900 Subject: [PATCH 24/37] Lint --- pkg/limit/limiter_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index ee510feda..7d1b8e186 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/yorkie-team/yorkie/pkg/limit" ) From dbf0817c1bd927491088c3263b7e22077aa0a5b2 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Tue, 11 Mar 2025 12:12:13 +0900 Subject: [PATCH 25/37] Check Event Webhook Requirement --- server/packs/packs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/packs/packs.go b/server/packs/packs.go index 5d6601461..7ca19c5c7 100644 --- a/server/packs/packs.go +++ b/server/packs/packs.go @@ -198,7 +198,7 @@ func PushPull( }, ) - if reqPack.OperationsLen() > 0 { + if reqPack.OperationsLen() > 0 && project.RequireEventWebhook(events.DocRootChangedEvent.WebhookType()) { info := types.NewEventWebhookInfo( docRefKey, events.DocRootChangedEvent.WebhookType(), From 38f408d993f53c869c0bbed1e0173863a2e5657c Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Tue, 11 Mar 2025 12:13:46 +0900 Subject: [PATCH 26/37] Decrease Throttle window and Debounce time --- server/backend/webhook/manager.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/backend/webhook/manager.go b/server/backend/webhook/manager.go index 03f3a4586..c2e106cde 100644 --- a/server/backend/webhook/manager.go +++ b/server/backend/webhook/manager.go @@ -31,9 +31,10 @@ import ( ) const ( + // TODO(window9u): Consider making this parameter configurable via CLI. expireInterval = 100 * time.Millisecond - throttleWindow = 5 * time.Second - debouncingTime = 3 * time.Second + throttleWindow = 1 * time.Second + debouncingTime = 1 * time.Second expireBatchSize = 100 ) From 01f655ef71b60b84a4433bf9bcd530b00de01d70 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Tue, 11 Mar 2025 12:14:25 +0900 Subject: [PATCH 27/37] Refactor event webhook test as spec changed --- test/integration/event_webhook_test.go | 248 +++++++++++++++++++------ 1 file changed, 189 insertions(+), 59 deletions(-) diff --git a/test/integration/event_webhook_test.go b/test/integration/event_webhook_test.go index 50e932727..922898675 100644 --- a/test/integration/event_webhook_test.go +++ b/test/integration/event_webhook_test.go @@ -89,10 +89,6 @@ func newYorkieServer(t *testing.T, projectCacheTTL string) *server.Yorkie { assert.NoError(t, err) assert.NoError(t, svr.Start()) - t.Cleanup(func() { - assert.NoError(t, svr.Shutdown(true)) - }) - return svr } @@ -100,18 +96,18 @@ func newActivatedClient(t *testing.T, ctx context.Context, addr, publicKey strin cli, err := client.Dial(addr, client.WithAPIKey(publicKey)) assert.NoError(t, err) assert.NoError(t, cli.Activate(ctx)) - t.Cleanup(func() { - assert.NoError(t, cli.Deactivate(ctx)) - assert.NoError(t, cli.Close()) - }) return cli } func TestRegisterEventWebhook(t *testing.T) { + const ( + projectCacheTTL = 1 * time.Millisecond + waitWebhookReceived = 10 * time.Millisecond + ) + ctx := context.Background() // Set up yorkie server - projectCacheTTL := 1 * time.Millisecond svr := newYorkieServer(t, projectCacheTTL.String()) // Set up project @@ -130,9 +126,7 @@ func TestRegisterEventWebhook(t *testing.T) { "counter": json.NewCounter(0, crdt.LongCnt), }))) - waitWebhookReceived := 10 * time.Millisecond - - t.Run("register event webhook test", func(t *testing.T) { + t.Run("register and unregister event webhook test", func(t *testing.T) { // 01. Register event webhook prj, err := adminCli.UpdateProject(ctx, project.ID.String(), &types.UpdatableProjectFields{ EventWebhookURL: &userServer.URL, @@ -154,11 +148,9 @@ func TestRegisterEventWebhook(t *testing.T) { assert.NoError(t, cli.Sync(ctx)) time.Sleep(waitWebhookReceived) assert.Equal(t, prev+1, atomic.LoadInt32(getReqCnt)) - }) - t.Run("unregister event webhook test", func(t *testing.T) { - // 01. Unregister event webhook - prj, err := adminCli.UpdateProject(ctx, project.ID.String(), &types.UpdatableProjectFields{ + // 04. Unregister event webhook + prj, err = adminCli.UpdateProject(ctx, project.ID.String(), &types.UpdatableProjectFields{ EventWebhookURL: &userServer.URL, EventWebhookEvents: &[]string{}, }) @@ -166,72 +158,126 @@ func TestRegisterEventWebhook(t *testing.T) { assert.Equal(t, userServer.URL, prj.EventWebhookURL) assert.Equal(t, 0, len(prj.EventWebhookEvents)) - // 02. Wait project cache expired + // 05. Wait project cache expired time.Sleep(projectCacheTTL) - // 03. Check webhook doesn't trigger - prev := atomic.LoadInt32(getReqCnt) + // 06. Check webhook doesn't trigger + prev = atomic.LoadInt32(getReqCnt) assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { root.GetCounter("counter").Increase(1) return nil })) assert.NoError(t, cli.Sync(ctx)) - // 04. Wait webhook received - time.Sleep(waitWebhookReceived) + // 07. Wait webhook received + assert.NoError(t, svr.Shutdown(true)) assert.Equal(t, prev, atomic.LoadInt32(getReqCnt)) }) } func TestDocRootChangedEventWebhook(t *testing.T) { - ctx := context.Background() + const waitWebhookReceived = 10 * time.Millisecond + t.Run("root element changed test", func(t *testing.T) { + ctx := context.Background() - svr := newYorkieServer(t, "default") - adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) + svr := newYorkieServer(t, "default") + adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) - project, err := adminCli.CreateProject(ctx, "doc-root-changed-event-webhook") - assert.NoError(t, err) + project, err := adminCli.CreateProject(ctx, "doc-root-changed-event-webhook") + assert.NoError(t, err) - doc := document.New(helper.TestDocKey(t)) - userServer, getReqCnt := newWebhookServer(t, project.SecretKey, doc.Key().String()) + doc := document.New(helper.TestDocKey(t)) + userServer, getReqCnt := newWebhookServer(t, project.SecretKey, doc.Key().String()) - project.EventWebhookURL = userServer.URL - _, err = adminCli.UpdateProject(ctx, project.ID.String(), &types.UpdatableProjectFields{ - EventWebhookURL: &project.EventWebhookURL, - EventWebhookEvents: &[]string{string(types.DocRootChanged)}, - }) - assert.NoError(t, err) + project.EventWebhookURL = userServer.URL + _, err = adminCli.UpdateProject(ctx, project.ID.String(), &types.UpdatableProjectFields{ + EventWebhookURL: &project.EventWebhookURL, + EventWebhookEvents: &[]string{string(types.DocRootChanged)}, + }) + assert.NoError(t, err) - cli := newActivatedClient(t, ctx, svr.RPCAddr(), project.PublicKey) + cli := newActivatedClient(t, ctx, svr.RPCAddr(), project.PublicKey) - assert.NoError(t, cli.Attach(ctx, doc, client.WithInitialRoot(map[string]any{ - "counter": json.NewCounter(0, crdt.LongCnt), - }))) + assert.NoError(t, cli.Attach(ctx, doc, client.WithInitialRoot(map[string]any{ + "counter": json.NewCounter(0, crdt.LongCnt), + }))) + assert.NoError(t, cli.Sync(ctx)) + time.Sleep(waitWebhookReceived) - waitWebhookReceived := 10 * time.Millisecond - t.Run("root element changed test", func(t *testing.T) { prev := atomic.LoadInt32(getReqCnt) assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { root.GetCounter("counter").Increase(1) return nil })) assert.NoError(t, cli.Sync(ctx)) - time.Sleep(waitWebhookReceived) + assert.NoError(t, svr.Shutdown(true)) assert.Equal(t, prev+1, atomic.LoadInt32(getReqCnt)) }) t.Run("presence changed test", func(t *testing.T) { + ctx := context.Background() + + svr := newYorkieServer(t, "default") + adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) + + project, err := adminCli.CreateProject(ctx, "presence-changed-event-webhook") + assert.NoError(t, err) + + doc := document.New(helper.TestDocKey(t)) + userServer, getReqCnt := newWebhookServer(t, project.SecretKey, doc.Key().String()) + + project.EventWebhookURL = userServer.URL + _, err = adminCli.UpdateProject(ctx, project.ID.String(), &types.UpdatableProjectFields{ + EventWebhookURL: &project.EventWebhookURL, + EventWebhookEvents: &[]string{string(types.DocRootChanged)}, + }) + assert.NoError(t, err) + + cli := newActivatedClient(t, ctx, svr.RPCAddr(), project.PublicKey) + + assert.NoError(t, cli.Attach(ctx, doc, client.WithInitialRoot(map[string]any{ + "counter": json.NewCounter(0, crdt.LongCnt), + }))) + assert.NoError(t, cli.Sync(ctx)) + time.Sleep(waitWebhookReceived) + prev := atomic.LoadInt32(getReqCnt) assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { p.Set("update", "2") return nil })) assert.NoError(t, cli.Sync(ctx)) - time.Sleep(waitWebhookReceived) + assert.NoError(t, svr.Shutdown(true)) assert.Equal(t, prev, atomic.LoadInt32(getReqCnt)) }) t.Run("root element and presence changed test", func(t *testing.T) { + ctx := context.Background() + + svr := newYorkieServer(t, "default") + adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) + + project, err := adminCli.CreateProject(ctx, "root-presence-changed-event") + assert.NoError(t, err) + + doc := document.New(helper.TestDocKey(t)) + userServer, getReqCnt := newWebhookServer(t, project.SecretKey, doc.Key().String()) + + project.EventWebhookURL = userServer.URL + _, err = adminCli.UpdateProject(ctx, project.ID.String(), &types.UpdatableProjectFields{ + EventWebhookURL: &project.EventWebhookURL, + EventWebhookEvents: &[]string{string(types.DocRootChanged)}, + }) + assert.NoError(t, err) + + cli := newActivatedClient(t, ctx, svr.RPCAddr(), project.PublicKey) + + assert.NoError(t, cli.Attach(ctx, doc, client.WithInitialRoot(map[string]any{ + "counter": json.NewCounter(0, crdt.LongCnt), + }))) + assert.NoError(t, cli.Sync(ctx)) + time.Sleep(waitWebhookReceived) + prev := atomic.LoadInt32(getReqCnt) assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { p.Set("update", "3") @@ -239,19 +285,31 @@ func TestDocRootChangedEventWebhook(t *testing.T) { return nil })) assert.NoError(t, cli.Sync(ctx)) - time.Sleep(waitWebhookReceived) + assert.NoError(t, svr.Shutdown(true)) assert.Equal(t, prev+1, atomic.LoadInt32(getReqCnt)) }) } -func TestEventWebhookCache(t *testing.T) { +func TestEventWebhookThrottling(t *testing.T) { + const ( + webhookThrottleWindow = 1 * time.Second + debouncingTime = 1 * time.Second + expirationInterval = 100 * time.Millisecond + waitWebhookReceived = 10 * time.Millisecond + + numWindows = 2 + eventPerWindow = 30 + + testDuration = webhookThrottleWindow * time.Duration(numWindows) + eventInterval = webhookThrottleWindow / eventPerWindow + ) + ctx := context.Background() - webhookCacheTTL := 10 * time.Millisecond svr := newYorkieServer(t, "default") adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) - project, err := adminCli.CreateProject(ctx, "event-webhook-cache-webhook") + project, err := adminCli.CreateProject(ctx, "event-webhook-throttle-webhook") assert.NoError(t, err) doc := document.New(helper.TestDocKey(t)) @@ -267,34 +325,106 @@ func TestEventWebhookCache(t *testing.T) { "counter": json.NewCounter(0, crdt.LongCnt), }))) - waitWebhookReceived := 20 * time.Millisecond - t.Run("throttling event test", func(t *testing.T) { - t.Skip("remove this after implement advanced event timing control") - - expectedUpdates := 5 - testDuration := webhookCacheTTL * time.Duration(expectedUpdates) - interval := webhookCacheTTL / 10 - - ticker := time.NewTicker(interval) + t.Run("throttling Event Test", func(t *testing.T) { + ticker := time.NewTicker(eventInterval) defer ticker.Stop() timeCtx, cancel := context.WithTimeout(ctx, testDuration) defer cancel() - prevCnt := atomic.LoadInt32(getReqCnt) + initialReqCount := atomic.LoadInt32(getReqCnt) + // Trigger document updates repeatedly. for { select { case <-ticker.C: - assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + // Update the document: increase the counter. + err := doc.Update(func(root *json.Object, p *presence.Presence) error { root.GetCounter("counter").Increase(1) return nil - })) + }) + assert.NoError(t, err) assert.NoError(t, cli.Sync(ctx)) case <-timeCtx.Done(): + // Wait briefly to allow any pending webhook events to be received. time.Sleep(waitWebhookReceived) - assert.Equal(t, prevCnt+int32(expectedUpdates), atomic.LoadInt32(getReqCnt)) + // Expect the request count to have increased by the expected number of updates. + assert.Equal(t, initialReqCount+int32(numWindows), atomic.LoadInt32(getReqCnt)) + // Expect the trailing event webhook for eventual consistency. + time.Sleep(webhookThrottleWindow + debouncingTime + expirationInterval) + assert.Equal(t, initialReqCount+int32(numWindows+1), atomic.LoadInt32(getReqCnt)) + assert.NoError(t, svr.Shutdown(true)) + assert.Equal(t, initialReqCount+int32(numWindows+1), atomic.LoadInt32(getReqCnt)) return } } }) } + +func TestCloseEventManager(t *testing.T) { + const ( + webhookThrottleWindow = 1 * time.Second + debouncingTime = 1 * time.Second + expirationInterval = 100 * time.Millisecond + waitWebhookReceived = 10 * time.Millisecond + ) + + ctx := context.Background() + + svr, err := server.New(helper.TestConfig()) + assert.NoError(t, err) + assert.NoError(t, svr.Start()) + adminCli := helper.CreateAdminCli(t, svr.RPCAddr()) + + project, err := adminCli.CreateProject(ctx, "close-event-webhook-webhook") + assert.NoError(t, err) + + doc := document.New(helper.TestDocKey(t)) + userServer, getReqCnt := newWebhookServer(t, project.SecretKey, doc.Key().String()) + _, err = adminCli.UpdateProject(ctx, project.ID.String(), &types.UpdatableProjectFields{ + EventWebhookURL: &userServer.URL, + EventWebhookEvents: &[]string{string(types.DocRootChanged)}, + }) + assert.NoError(t, err) + + cli, err := client.Dial(svr.RPCAddr(), client.WithAPIKey(project.PublicKey)) + assert.NoError(t, err) + assert.NoError(t, cli.Activate(ctx)) + assert.NoError(t, cli.Attach(ctx, doc, client.WithInitialRoot(map[string]any{ + "counter": json.NewCounter(0, crdt.LongCnt), + }))) + + t.Run("Force flush event when server shutdown Test", func(t *testing.T) { + // this triggers webhook directly. + prev := atomic.LoadInt32(getReqCnt) + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.GetCounter("counter").Increase(1) + return nil + })) + assert.NoError(t, cli.Sync(ctx)) + time.Sleep(waitWebhookReceived) + assert.Equal(t, prev+1, atomic.LoadInt32(getReqCnt)) + + // this is queued and will be flushed by closing server + prev = atomic.LoadInt32(getReqCnt) + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.GetCounter("counter").Increase(1) + return nil + })) + assert.NoError(t, cli.Sync(ctx)) + assert.Equal(t, prev, atomic.LoadInt32(getReqCnt)) + + done := make(chan struct{}) + go func() { + assert.NoError(t, svr.Shutdown(true)) + done <- struct{}{} + }() + + select { + case <-done: + assert.Equal(t, prev+1, atomic.LoadInt32(getReqCnt)) + case <-time.After(webhookThrottleWindow + debouncingTime + expirationInterval): + assert.Equal(t, prev+1, atomic.LoadInt32(getReqCnt)) + assert.Fail(t, "closing timeout") + } + }) +} From acd606c5b618cdb39ae3346bb85a30fab5eb9472 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Tue, 11 Mar 2025 14:44:04 +0900 Subject: [PATCH 28/37] Wait previous debouncing before flushing --- pkg/limit/limiter.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index 33b4f5415..49f55464c 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -59,6 +59,7 @@ func NewLimiter[K comparable](expireNum int, expire, throttle, debouncing time.D } // Start the background expiration process. + lim.wg.Add(1) go lim.expirationLoop() return lim } @@ -108,7 +109,10 @@ func (l *Limiter[K]) Allow(key K, callback func()) bool { // expirationLoop runs in a separate goroutine to periodically remove expired entries. func (l *Limiter[K]) expirationLoop() { ticker := time.NewTicker(l.expireInterval) - defer ticker.Stop() + defer func() { + ticker.Stop() + l.wg.Done() + }() for { select { @@ -164,6 +168,7 @@ func (l *Limiter[K]) runDebounce(entries []*limiterEntry[K]) { // Close terminates the expiration loop and cleans up resources. func (l *Limiter[K]) Close() { close(l.closing) + l.wg.Wait() for l.evictionList.Len() > 0 { expiredEntries := l.collectEntries(false) From 73b6c2539849b9cd643c32a602f652ac62b89e34 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Tue, 11 Mar 2025 14:54:07 +0900 Subject: [PATCH 29/37] Refactor tests --- pkg/limit/limiter_test.go | 53 ++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index 7d1b8e186..aaa6e4cc7 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -20,6 +20,7 @@ package limit_test import ( "context" "fmt" + "sync" "sync/atomic" "testing" "time" @@ -37,7 +38,7 @@ func TestSynchronousExecution(t *testing.T) { throttleWindow = 100 * time.Millisecond debouncingTime = 100 * time.Millisecond waitingTime = expireInterval + throttleWindow + debouncingTime - numExecute = 10000 + numExecute = 1000 ) t.Run("Single Call", func(t *testing.T) { @@ -79,12 +80,12 @@ func TestSynchronousExecution(t *testing.T) { // TestConcurrentExecution verifies the throttler behavior under concurrent execution scenarios. func TestConcurrentExecution(t *testing.T) { const ( - expireBatchSize = 10000 + expireBatchSize = 100 expireInterval = 10 * time.Millisecond throttleWindow = 100 * time.Millisecond debouncingTime = 100 * time.Millisecond waitingTime = expireInterval + throttleWindow + debouncingTime - numExecute = 10000 + numExecute = 1000 ) t.Run("Concurrent Calls: Single Immediate and Trailing Execution", func(t *testing.T) { @@ -95,13 +96,17 @@ func TestConcurrentExecution(t *testing.T) { atomic.AddInt32(&callCount, 1) } + wg := sync.WaitGroup{} for range numExecute { + wg.Add(1) go func() { + defer wg.Done() if lim.Allow("key", callback) { callback() } }() } + wg.Wait() time.Sleep(waitingTime) // Expect one immediate and one trailing callback execution. @@ -135,9 +140,9 @@ func TestConcurrentExecution(t *testing.T) { t.Run("Continuous Event Stream Throttling", func(t *testing.T) { const ( - numWindows = 10 // Number of throttle windows. - eventPerWindow = 1000 // Number of events triggered within each window. - numRoutines = 100 // Number of concurrent calls per tick. + numWindows = 3 // Number of throttle windows. + eventPerWindow = 100 // Number of events triggered within each window. + numExecutes = 100 // Number of concurrent calls per tick. totalDuration = throttleWindow * time.Duration(numWindows) // Total simulation duration. eventInterval = throttleWindow / time.Duration(eventPerWindow) // Interval between events. ) @@ -159,12 +164,10 @@ func TestConcurrentExecution(t *testing.T) { select { case <-ticker.C: // Each tick triggers multiple concurrent calls. - for range numRoutines { - go func() { - if lim.Allow("key", callback) { - callback() - } - }() + for range numExecutes { + if lim.Allow("key", callback) { + callback() + } } case <-timeCtx.Done(): // Allow time for any trailing callback to execute. @@ -182,12 +185,12 @@ func TestConcurrentExecution(t *testing.T) { // TestBatchExpiration verifies that the expiration loop processes expired entries in batches. func TestBatchExpiration(t *testing.T) { const ( - expireInterval = 10 * time.Millisecond + expireInterval = 100 * time.Millisecond throttleWindow = 50 * time.Millisecond debouncingTime = 50 * time.Millisecond - expireBatchSize = 100 - batchNum int32 = 10 + expireBatchSize = 10 + batchNum int32 = 3 totalKeys = expireBatchSize * batchNum // create more keys than one batch ) @@ -211,9 +214,8 @@ func TestBatchExpiration(t *testing.T) { } assert.Equal(t, totalKeys, atomic.LoadInt32(&callCount)) - - time.Sleep(throttleWindow + debouncingTime + expireInterval/2) - for i := int32(1); i <= batchNum; i++ { + time.Sleep(expireInterval / 2) + for i := range batchNum { assert.Equal(t, totalKeys+expireBatchSize*i, atomic.LoadInt32(&callCount)) time.Sleep(expireInterval) } @@ -240,7 +242,18 @@ func TestBatchExpiration(t *testing.T) { } assert.Equal(t, totalKeys, atomic.LoadInt32(&callCount)) - lim.Close() - assert.Equal(t, totalKeys*2, atomic.LoadInt32(&callCount)) + + done := make(chan struct{}) + go func() { + lim.Close() + done <- struct{}{} + }() + select { + case <-done: + assert.Equal(t, totalKeys*2, atomic.LoadInt32(&callCount)) + case <-time.After(expireInterval): + assert.Equal(t, totalKeys*2, atomic.LoadInt32(&callCount)) + assert.Fail(t, "close timeout") + } }) } From e794ac64ba964859abecc7a5685a67aa8a4138ab Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Tue, 11 Mar 2025 15:14:27 +0900 Subject: [PATCH 30/37] Add wait group in test --- pkg/limit/limiter_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index aaa6e4cc7..5acd485de 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -122,14 +122,18 @@ func TestConcurrentExecution(t *testing.T) { atomic.AddInt32(&callCount, 1) } + wg := sync.WaitGroup{} for i := range numExecute { i := i + wg.Add(1) go func() { + defer wg.Done() if lim.Allow(i, callback) { callback() } }() } + wg.Wait() time.Sleep(waitingTime) // For different keys, every call is immediate so expect numExecute callbacks. From 7d05a69ca05f3dd2371dd29a8672b2b241dd70d3 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 12 Mar 2025 11:25:28 +0900 Subject: [PATCH 31/37] Add detail stimulation test --- pkg/limit/limiter_test.go | 218 ++++++++++++++++++++++++++++++++++---- 1 file changed, 197 insertions(+), 21 deletions(-) diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index 5acd485de..6977bc77b 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -30,50 +30,207 @@ import ( "github.com/yorkie-team/yorkie/pkg/limit" ) -// TestSynchronousExecution verifies the behavior of synchronous calls to the throttler. -func TestSynchronousExecution(t *testing.T) { +// occurs encapsulates a slice of integers with a mutex for concurrent access. +type occurs struct { + array []int + mu sync.Mutex +} + +// add appends a value to the array. +func (o *occurs) add(e int) { + o.mu.Lock() + defer o.mu.Unlock() + o.array = append(o.array, e) +} + +// len returns the length of the array. +func (o *occurs) len() int { + o.mu.Lock() + defer o.mu.Unlock() + return len(o.array) +} + +// get returns the element at the specified index. +func (o *occurs) get(index int) int { + o.mu.Lock() + defer o.mu.Unlock() + return o.array[index] +} + +// TestThrottlerBehavior verifies the behavior of synchronous calls to the throttler. +// You can refer to the visualization in https://github.com/yorkie-team/yorkie/pull/1166. +func TestThrottlerBehavior(t *testing.T) { const ( expireBatchSize = 100 expireInterval = 10 * time.Millisecond throttleWindow = 100 * time.Millisecond debouncingTime = 100 * time.Millisecond waitingTime = expireInterval + throttleWindow + debouncingTime - numExecute = 1000 ) - t.Run("Single Call", func(t *testing.T) { + // Test case: "e1" -> e1 occurs + t.Run("e1", func(t *testing.T) { lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) + o := &occurs{} + + e1 := func() { o.add(1) } + if lim.Allow("key", e1) { + e1() + } + // Immediately after execution, the callback should have been invoked. + assert.Equal(t, 1, o.get(0)) + // After waiting, no additional invocation should occur. + time.Sleep(waitingTime) + assert.Equal(t, 1, o.len()) + lim.Close() + }) + + // Test case: "e1 e2" -> e1 then e2 occurs + t.Run("e1 e2", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) + o := &occurs{} + + e1 := func() { o.add(1) } + if lim.Allow("key", e1) { + e1() } + // First callback is executed directly. + assert.Equal(t, 1, o.get(0)) - if lim.Allow("key", callback) { - callback() + e2 := func() { o.add(2) } + if lim.Allow("key", e2) { + e2() } + + // At this point, only the immediate callback should have occurred. + assert.Equal(t, 1, o.len()) time.Sleep(waitingTime) - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) + + // After waiting, the deferred callback should be executed. + assert.Equal(t, 2, o.len()) + assert.Equal(t, 2, o.get(1)) lim.Close() - assert.Equal(t, int32(1), atomic.LoadInt32(&callCount)) }) - t.Run("Multiple Synchronous Calls with Trailing Debounce", func(t *testing.T) { + // Test case: "e1 e2 e3" -> e1 immediately and e3 deferred + t.Run("e1 e2 e3", func(t *testing.T) { lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) + o := &occurs{} + + e1 := func() { o.add(1) } + if lim.Allow("key", e1) { + e1() } + // First callback is executed immediately. + assert.Equal(t, 1, o.get(0)) - for range numExecute { - if lim.Allow("key", callback) { - callback() - } + e2 := func() { o.add(2) } + if lim.Allow("key", e2) { + e2() + } + e3 := func() { o.add(3) } + if lim.Allow("key", e3) { + e3() } + // Only the immediate callback should have been executed so far. + assert.Equal(t, 1, o.len()) time.Sleep(waitingTime) - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + + // After waiting, the latest callback (e3) is executed. + assert.Equal(t, 2, o.len()) + assert.Equal(t, 3, o.get(1)) + lim.Close() + }) + + // Test case: "/ e1 e2 e3 / e4" -> e1 immediately then e4 immediately when allowed + t.Run("/ e1 e2 e3 / e4", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) + o := &occurs{} + + e1 := func() { o.add(1) } + if lim.Allow("key", e1) { + e1() + } + // Immediate execution for the first callback. + assert.Equal(t, 1, o.get(0)) + + e2 := func() { o.add(2) } + if lim.Allow("key", e2) { + e2() + } + e3 := func() { o.add(3) } + if lim.Allow("key", e3) { + e3() + } + + // Still, only the immediate callback should have been executed. + assert.Equal(t, 1, o.len()) + + // Wait for part of the throttle window; deferred callbacks are not yet flushed. + time.Sleep(throttleWindow + debouncingTime/2) + assert.Equal(t, 1, o.len()) + + e4 := func() { o.add(4) } + if lim.Allow("key", e4) { + e4() + } + + // The new callback should now be executed immediately. + assert.Equal(t, 2, o.len()) + assert.Equal(t, 4, o.get(1)) + time.Sleep(waitingTime) + // No further callbacks should be executed after waiting. + assert.Equal(t, 2, o.len()) + lim.Close() + }) + + // Test case: "/ e1 e2 e3 / e4 e5" -> e1, then e4 immediately, then e5 deferred + t.Run("/ e1 e2 e3 / e4 e5", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) + o := &occurs{} + + // e1 occurs directly. + e1 := func() { o.add(1) } + if lim.Allow("key", e1) { + e1() + } + assert.Equal(t, 1, o.get(0)) + + // e2 is saved. + e2 := func() { o.add(2) } + if lim.Allow("key", e2) { + e2() + } + // e3 replaces e2. + e3 := func() { o.add(3) } + if lim.Allow("key", e3) { + e3() + } + assert.Equal(t, 1, o.len()) + time.Sleep(throttleWindow + debouncingTime/2) + assert.Equal(t, 1, o.len()) + + // Before flushing e3, e4 occurs so e3 is skipped. + e4 := func() { o.add(4) } + if lim.Allow("key", e4) { + e4() + } + assert.Equal(t, 2, o.len()) + assert.Equal(t, 4, o.get(1)) + + // e5 meets limit so it is saved. + e5 := func() { o.add(5) } + if lim.Allow("key", e5) { + e5() + } + assert.Equal(t, 2, o.len()) + + // And flushed when it expires. + time.Sleep(waitingTime) + assert.Equal(t, 3, o.len()) + assert.Equal(t, 5, o.get(2)) lim.Close() - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) }) } @@ -88,6 +245,25 @@ func TestConcurrentExecution(t *testing.T) { numExecute = 1000 ) + t.Run("Multiple Synchronous Calls with Trailing Debounce", func(t *testing.T) { + lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) + var callCount int32 + callback := func() { + atomic.AddInt32(&callCount, 1) + } + + for range numExecute { + if lim.Allow("key", callback) { + callback() + } + } + + time.Sleep(waitingTime) + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + lim.Close() + assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + }) + t.Run("Concurrent Calls: Single Immediate and Trailing Execution", func(t *testing.T) { lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) From 261ef28075dbe8b9aa3e7bc4cba836c050f1e6ab Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 12 Mar 2025 13:02:09 +0900 Subject: [PATCH 32/37] refactor test with `occurs` type --- pkg/limit/limiter_test.go | 83 +++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index 6977bc77b..f703ff1fa 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -21,7 +21,6 @@ import ( "context" "fmt" "sync" - "sync/atomic" "testing" "time" @@ -65,7 +64,8 @@ func TestThrottlerBehavior(t *testing.T) { expireInterval = 10 * time.Millisecond throttleWindow = 100 * time.Millisecond debouncingTime = 100 * time.Millisecond - waitingTime = expireInterval + throttleWindow + debouncingTime + executeTime = 5 * time.Millisecond + waitingTime = expireInterval + throttleWindow + debouncingTime + executeTime ) // Test case: "e1" -> e1 occurs @@ -241,36 +241,33 @@ func TestConcurrentExecution(t *testing.T) { expireInterval = 10 * time.Millisecond throttleWindow = 100 * time.Millisecond debouncingTime = 100 * time.Millisecond - waitingTime = expireInterval + throttleWindow + debouncingTime + executeTime = 5 * time.Millisecond + waitingTime = expireInterval + throttleWindow + debouncingTime + executeTime numExecute = 1000 ) t.Run("Multiple Synchronous Calls with Trailing Debounce", func(t *testing.T) { lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } + o := occurs{} + callback := func() { o.add(1) } for range numExecute { if lim.Allow("key", callback) { callback() } } + assert.Equal(t, 1, o.len()) time.Sleep(waitingTime) - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + assert.Equal(t, 2, o.len()) lim.Close() - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + assert.Equal(t, 2, o.len()) }) t.Run("Concurrent Calls: Single Immediate and Trailing Execution", func(t *testing.T) { lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) - - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) - } + o := occurs{} + callback := func() { o.add(1) } wg := sync.WaitGroup{} for range numExecute { @@ -283,20 +280,20 @@ func TestConcurrentExecution(t *testing.T) { }() } wg.Wait() + assert.Equal(t, 1, o.len()) time.Sleep(waitingTime) - // Expect one immediate and one trailing callback execution. - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + assert.Equal(t, 2, o.len()) lim.Close() - assert.Equal(t, int32(2), atomic.LoadInt32(&callCount)) + assert.Equal(t, 2, o.len()) }) t.Run("Concurrent Calls with Different Keys", func(t *testing.T) { lim := limit.NewLimiter[int](expireBatchSize, expireInterval, throttleWindow, debouncingTime) - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) + o := occurs{ + array: make([]int, 0, numExecute*2), } + callback := func() { o.add(1) } wg := sync.WaitGroup{} for i := range numExecute { @@ -312,10 +309,9 @@ func TestConcurrentExecution(t *testing.T) { wg.Wait() time.Sleep(waitingTime) - // For different keys, every call is immediate so expect numExecute callbacks. - assert.Equal(t, int32(numExecute), atomic.LoadInt32(&callCount)) + assert.Equal(t, numExecute, o.len()) lim.Close() - assert.Equal(t, int32(numExecute), atomic.LoadInt32(&callCount)) + assert.Equal(t, numExecute, o.len()) }) t.Run("Continuous Event Stream Throttling", func(t *testing.T) { @@ -329,10 +325,10 @@ func TestConcurrentExecution(t *testing.T) { lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) + o := occurs{ + array: make([]int, 0, numWindows+1), } + callback := func() { o.add(1) } ticker := time.NewTicker(eventInterval) defer ticker.Stop() @@ -350,12 +346,13 @@ func TestConcurrentExecution(t *testing.T) { } } case <-timeCtx.Done(): + assert.Equal(t, numWindows, o.len()) // Allow time for any trailing callback to execute. time.Sleep(waitingTime) // Expect one callback per throttle window plus one additional trailing call. - assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) + assert.Equal(t, numWindows+1, o.len()) lim.Close() - assert.Equal(t, int32(numWindows+1), atomic.LoadInt32(&callCount)) + assert.Equal(t, numWindows+1, o.len()) return } } @@ -369,18 +366,18 @@ func TestBatchExpiration(t *testing.T) { throttleWindow = 50 * time.Millisecond debouncingTime = 50 * time.Millisecond - expireBatchSize = 10 - batchNum int32 = 3 + expireBatchSize = 10 + batchNum = 3 totalKeys = expireBatchSize * batchNum // create more keys than one batch ) t.Run("Process Expire Batch", func(t *testing.T) { lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) + o := occurs{ + array: make([]int, 0, totalKeys*2), } + callback := func() { o.add(1) } // For each key: first call executes immediately, second call schedules a debounced callback. for i := range totalKeys { @@ -393,23 +390,23 @@ func TestBatchExpiration(t *testing.T) { lim.Allow(key, callback) } - assert.Equal(t, totalKeys, atomic.LoadInt32(&callCount)) + assert.Equal(t, totalKeys, o.len()) time.Sleep(expireInterval / 2) for i := range batchNum { - assert.Equal(t, totalKeys+expireBatchSize*i, atomic.LoadInt32(&callCount)) + assert.Equal(t, totalKeys+expireBatchSize*i, o.len()) time.Sleep(expireInterval) } - assert.Equal(t, totalKeys+expireBatchSize*batchNum, atomic.LoadInt32(&callCount)) + assert.Equal(t, totalKeys+expireBatchSize*batchNum, o.len()) lim.Close() - assert.Equal(t, totalKeys+expireBatchSize*batchNum, atomic.LoadInt32(&callCount)) + assert.Equal(t, totalKeys+expireBatchSize*batchNum, o.len()) }) t.Run("Force Close Expired", func(t *testing.T) { lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) - var callCount int32 - callback := func() { - atomic.AddInt32(&callCount, 1) + o := occurs{ + array: make([]int, 0, totalKeys*2), } + callback := func() { o.add(1) } for i := range totalKeys { key := fmt.Sprintf("key-%d", i) @@ -421,7 +418,7 @@ func TestBatchExpiration(t *testing.T) { lim.Allow(key, callback) } - assert.Equal(t, totalKeys, atomic.LoadInt32(&callCount)) + assert.Equal(t, totalKeys, o.len()) done := make(chan struct{}) go func() { @@ -430,9 +427,9 @@ func TestBatchExpiration(t *testing.T) { }() select { case <-done: - assert.Equal(t, totalKeys*2, atomic.LoadInt32(&callCount)) + assert.Equal(t, totalKeys+expireBatchSize*batchNum, o.len()) case <-time.After(expireInterval): - assert.Equal(t, totalKeys*2, atomic.LoadInt32(&callCount)) + assert.Equal(t, totalKeys+expireBatchSize*batchNum, o.len()) assert.Fail(t, "close timeout") } }) From b14129368eaddef9cdf1d3202767eae9d7633ddb Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 12 Mar 2025 13:59:35 +0900 Subject: [PATCH 33/37] Add comment --- pkg/limit/limiter.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/limit/limiter.go b/pkg/limit/limiter.go index 49f55464c..999a8d746 100644 --- a/pkg/limit/limiter.go +++ b/pkg/limit/limiter.go @@ -168,6 +168,8 @@ func (l *Limiter[K]) runDebounce(entries []*limiterEntry[K]) { // Close terminates the expiration loop and cleans up resources. func (l *Limiter[K]) Close() { close(l.closing) + + // Wait for all previous expiration job done. l.wg.Wait() for l.evictionList.Len() > 0 { From 526b37086b3b56314d55d04a9054cfce6286bae9 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 12 Mar 2025 14:01:28 +0900 Subject: [PATCH 34/37] Set `verifySignature` to helper function --- pkg/webhook/client_test.go | 20 ++------------------ test/helper/helper.go | 16 ++++++++++++++++ test/integration/event_webhook_test.go | 18 +----------------- 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/pkg/webhook/client_test.go b/pkg/webhook/client_test.go index b0afa1a34..7bc48da98 100644 --- a/pkg/webhook/client_test.go +++ b/pkg/webhook/client_test.go @@ -2,12 +2,7 @@ package webhook_test import ( "context" - "crypto/hmac" - "crypto/sha256" - "encoding/hex" "encoding/json" - "errors" - "fmt" "io" "net/http" "net/http/httptest" @@ -18,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/yorkie-team/yorkie/pkg/webhook" + "github.com/yorkie-team/yorkie/test/helper" ) // testRequest is a simple request type for demonstration. @@ -30,18 +26,6 @@ type testResponse struct { Greeting string `json:"greeting"` } -// verifySignature verifies that the HMAC signature in the header matches the expected value. -func verifySignature(signatureHeader, secret string, body []byte) error { - mac := hmac.New(sha256.New, []byte(secret)) - mac.Write(body) - expectedSig := hex.EncodeToString(mac.Sum(nil)) - expectedSigHeader := fmt.Sprintf("sha256=%s", expectedSig) - if !hmac.Equal([]byte(signatureHeader), []byte(expectedSigHeader)) { - return errors.New("signature validation failed") - } - return nil -} - // newHMACTestServer creates a new httptest.Server that verifies the HMAC signature. // It returns a valid JSON response if the signature is correct. func newHMACTestServer(t *testing.T, validSecret string, responseData testResponse) *httptest.Server { @@ -58,7 +42,7 @@ func newHMACTestServer(t *testing.T, validSecret string, responseData testRespon return } - if err := verifySignature(signatureHeader, validSecret, bodyBytes); err != nil { + if err := helper.VerifySignature(signatureHeader, validSecret, bodyBytes); err != nil { http.Error(w, "forbidden", http.StatusForbidden) return } diff --git a/test/helper/helper.go b/test/helper/helper.go index 00d5c45ef..8218e2b84 100644 --- a/test/helper/helper.go +++ b/test/helper/helper.go @@ -19,6 +19,10 @@ package helper import ( "context" + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "errors" "fmt" "log" "net" @@ -578,3 +582,15 @@ func CleanupClients(b *testing.B, clients []*client.Client) { assert.NoError(b, c.Close()) } } + +// VerifySignature verifies that the HMAC signature in the header matches the expected value. +func VerifySignature(signatureHeader, secret string, body []byte) error { + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write(body) + expectedSig := hex.EncodeToString(mac.Sum(nil)) + expectedSigHeader := fmt.Sprintf("sha256=%s", expectedSig) + if !hmac.Equal([]byte(signatureHeader), []byte(expectedSigHeader)) { + return errors.New("signature validation failed") + } + return nil +} diff --git a/test/integration/event_webhook_test.go b/test/integration/event_webhook_test.go index 922898675..b8a27981a 100644 --- a/test/integration/event_webhook_test.go +++ b/test/integration/event_webhook_test.go @@ -20,12 +20,7 @@ package integration import ( "context" - "crypto/hmac" - "crypto/sha256" - "encoding/hex" gojson "encoding/json" - "errors" - "fmt" "io" "net/http" "net/http/httptest" @@ -45,17 +40,6 @@ import ( "github.com/yorkie-team/yorkie/test/helper" ) -func verifySignature(signatureHeader, secret string, body []byte) error { - mac := hmac.New(sha256.New, []byte(secret)) - mac.Write(body) - expectedSig := hex.EncodeToString(mac.Sum(nil)) - expectedSigHeader := fmt.Sprintf("sha256=%s", expectedSig) - if !hmac.Equal([]byte(signatureHeader), []byte(expectedSigHeader)) { - return errors.New("signature validation failed") - } - return nil -} - func newWebhookServer(t *testing.T, secretKey, docKey string) (*httptest.Server, *int32) { var reqCnt int32 @@ -65,7 +49,7 @@ func newWebhookServer(t *testing.T, secretKey, docKey string) (*httptest.Server, assert.NotZero(t, len(signatureHeader)) body, err := io.ReadAll(r.Body) assert.NoError(t, err) - assert.NoError(t, verifySignature(signatureHeader, secretKey, body)) + assert.NoError(t, helper.VerifySignature(signatureHeader, secretKey, body)) req := &types.EventWebhookRequest{} assert.NoError(t, gojson.Unmarshal(body, req)) From 48e7f651bec5b9830b3dae153d9dcc3abdfae5fd Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 12 Mar 2025 14:01:56 +0900 Subject: [PATCH 35/37] Add bench test for webhook --- server/backend/webhook/manager.go | 8 +- test/bench/webhook_bench_test.go | 163 ++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 test/bench/webhook_bench_test.go diff --git a/server/backend/webhook/manager.go b/server/backend/webhook/manager.go index c2e106cde..314b3f21f 100644 --- a/server/backend/webhook/manager.go +++ b/server/backend/webhook/manager.go @@ -61,14 +61,14 @@ func NewManager(cli *webhook.Client[types.EventWebhookRequest, int]) *Manager { // It uses rate limiting to debounce multiple events within a short period. func (m *Manager) Send(ctx context.Context, info types.EventWebhookInfo) error { callback := func() { - if err := sendWebhook(ctx, m.webhookClient, info.EventRefKey.EventWebhookType, info.Attribute); err != nil { + if err := SendWebhook(ctx, m.webhookClient, info.EventRefKey.EventWebhookType, info.Attribute); err != nil { logging.From(ctx).Error(err) } } // If allowed immediately, invoke the callback. if allowed := m.limiter.Allow(info.EventRefKey, callback); allowed { - return sendWebhook(ctx, m.webhookClient, info.EventRefKey.EventWebhookType, info.Attribute) + return SendWebhook(ctx, m.webhookClient, info.EventRefKey.EventWebhookType, info.Attribute) } return nil } @@ -78,9 +78,9 @@ func (m *Manager) Close() { m.limiter.Close() } -// sendWebhook sends the webhook event using the provided client. +// SendWebhook sends the webhook event using the provided client. // It builds the request body and checks for a successful HTTP response. -func sendWebhook( +func SendWebhook( ctx context.Context, cli *webhook.Client[types.EventWebhookRequest, int], event types.EventWebhookType, diff --git a/test/bench/webhook_bench_test.go b/test/bench/webhook_bench_test.go new file mode 100644 index 000000000..4e9922641 --- /dev/null +++ b/test/bench/webhook_bench_test.go @@ -0,0 +1,163 @@ +/* + * Copyright 2025 The Yorkie Authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package bench + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/yorkie-team/yorkie/api/types" + pkgwebhook "github.com/yorkie-team/yorkie/pkg/webhook" + "github.com/yorkie-team/yorkie/server/backend/webhook" +) + +// setupWebhookServer simulates an HTTP server for the benchmark. +func setupWebhookServer(t *testing.B, count int) []*httptest.Server { + servers := make([]*httptest.Server, 0, count) + for range count { + servers = append(servers, httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.NotEmpty(t, r.Header.Get("X-Signature-256")) + w.WriteHeader(http.StatusOK) + }))) + } + return servers +} + +func BenchmarkWebhook(b *testing.B) { + benches := []struct { + endpointNum int + webhookNum int + delay time.Duration + }{ + {endpointNum: 10, webhookNum: 10, delay: 0}, + {endpointNum: 10, webhookNum: 100, delay: 0}, + {endpointNum: 100, webhookNum: 10, delay: 0}, + {endpointNum: 100, webhookNum: 100, delay: 0}, + + {endpointNum: 10, webhookNum: 10, delay: 10 * time.Millisecond}, + {endpointNum: 10, webhookNum: 100, delay: 10 * time.Millisecond}, + {endpointNum: 100, webhookNum: 10, delay: 10 * time.Millisecond}, + {endpointNum: 100, webhookNum: 100, delay: 10 * time.Millisecond}, + } + + for _, bench := range benches { + tName := fmt.Sprintf( + "Send %d Webhooks to %d Endpoints with delay %s", + bench.webhookNum, + bench.endpointNum, + bench.delay.String(), + ) + b.Run(tName, func(b *testing.B) { + benchmarkSendWebhook(b, bench.webhookNum, bench.endpointNum, bench.delay) + }) + + tName = fmt.Sprintf( + "Send %d Webhooks to %d Endpoints limiter with delay %s", + bench.webhookNum, + bench.endpointNum, + bench.delay.String(), + ) + b.Run(tName, func(b *testing.B) { + benchmarkSendWebhookWithLimits(b, bench.webhookNum, bench.endpointNum, bench.delay) + }) + } +} + +func benchmarkSendWebhook(b *testing.B, webhookNum, endpointNum int, delay time.Duration) { + b.ReportAllocs() + const ( + docKey = "doc-key" + signingKey = "sign-key" + ) + endpoints := setupWebhookServer(b, endpointNum) + cli := pkgwebhook.NewClient[types.EventWebhookRequest, int]( + pkgwebhook.Options{ + MaxRetries: 0, + MinWaitInterval: 100 * time.Millisecond, + MaxWaitInterval: 100 * time.Millisecond, + RequestTimeout: 100 * time.Millisecond, + }, + ) + for range b.N { + for range webhookNum { + for i := range endpointNum { + err := webhook.SendWebhook( + context.Background(), + cli, + types.DocRootChanged, + types.WebhookAttribute{ + DocKey: docKey, + SigningKey: signingKey, + URL: endpoints[i].URL, + }, + ) + assert.NoError(b, err) + } + if delay > 0 { + time.Sleep(delay) + } + } + } +} + +func benchmarkSendWebhookWithLimits(b *testing.B, webhookNum, endpointNum int, delay time.Duration) { + b.ReportAllocs() + const ( + docKey = "doc-key" + signingKey = "sign-key" + ) + docRefKey := types.DocRefKey{ + DocID: "doc-id", + ProjectID: "prj-id", + } + + endpoints := setupWebhookServer(b, endpointNum) + cli := pkgwebhook.NewClient[types.EventWebhookRequest, int]( + pkgwebhook.Options{ + MaxRetries: 0, + MinWaitInterval: 100 * time.Millisecond, + MaxWaitInterval: 100 * time.Millisecond, + RequestTimeout: 100 * time.Millisecond, + }, + ) + manager := webhook.NewManager(cli) + for range b.N { + for range webhookNum { + for i := range endpointNum { + err := manager.Send( + context.Background(), + types.NewEventWebhookInfo( + docRefKey, + types.DocRootChanged, + signingKey, + endpoints[i].URL, + docKey, + ), + ) + assert.NoError(b, err) + } + if delay > 0 { + time.Sleep(delay) + } + } + } +} From f839b16945683ba237d80efa60c187d4f741adc3 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 12 Mar 2025 15:39:06 +0900 Subject: [PATCH 36/37] Refactor limit event stream test --- pkg/limit/limiter_test.go | 43 +++++++++++++-------------------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/pkg/limit/limiter_test.go b/pkg/limit/limiter_test.go index f703ff1fa..3e779bb9e 100644 --- a/pkg/limit/limiter_test.go +++ b/pkg/limit/limiter_test.go @@ -18,7 +18,6 @@ package limit_test import ( - "context" "fmt" "sync" "testing" @@ -316,11 +315,7 @@ func TestConcurrentExecution(t *testing.T) { t.Run("Continuous Event Stream Throttling", func(t *testing.T) { const ( - numWindows = 3 // Number of throttle windows. - eventPerWindow = 100 // Number of events triggered within each window. - numExecutes = 100 // Number of concurrent calls per tick. - totalDuration = throttleWindow * time.Duration(numWindows) // Total simulation duration. - eventInterval = throttleWindow / time.Duration(eventPerWindow) // Interval between events. + numWindows = 3 // Number of throttle windows. ) lim := limit.NewLimiter[string](expireBatchSize, expireInterval, throttleWindow, debouncingTime) @@ -328,33 +323,23 @@ func TestConcurrentExecution(t *testing.T) { o := occurs{ array: make([]int, 0, numWindows+1), } - callback := func() { o.add(1) } - - ticker := time.NewTicker(eventInterval) - defer ticker.Stop() - timeCtx, cancel := context.WithTimeout(context.Background(), totalDuration) - defer cancel() // Continuously trigger events until the simulation ends. - for { - select { - case <-ticker.C: - // Each tick triggers multiple concurrent calls. - for range numExecutes { - if lim.Allow("key", callback) { - callback() - } + for i := range numWindows { + callback := func() { o.add(i) } + for range numExecute { + if lim.Allow("key", callback) { + callback() } - case <-timeCtx.Done(): - assert.Equal(t, numWindows, o.len()) - // Allow time for any trailing callback to execute. - time.Sleep(waitingTime) - // Expect one callback per throttle window plus one additional trailing call. - assert.Equal(t, numWindows+1, o.len()) - lim.Close() - assert.Equal(t, numWindows+1, o.len()) - return } + time.Sleep(throttleWindow) + } + assert.Equal(t, numWindows, o.len()) + time.Sleep(waitingTime) + assert.Equal(t, numWindows+1, o.len()) + + for i := range numWindows { + assert.Equal(t, i, o.get(i)) } }) } From bd7c9308726a8947a2b35f0788ba8f306ba7d423 Mon Sep 17 00:00:00 2001 From: Changyu Moon Date: Wed, 12 Mar 2025 15:39:19 +0900 Subject: [PATCH 37/37] Lint --- test/bench/webhook_bench_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/test/bench/webhook_bench_test.go b/test/bench/webhook_bench_test.go index 4e9922641..a69e9178c 100644 --- a/test/bench/webhook_bench_test.go +++ b/test/bench/webhook_bench_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/yorkie-team/yorkie/api/types" pkgwebhook "github.com/yorkie-team/yorkie/pkg/webhook" "github.com/yorkie-team/yorkie/server/backend/webhook"