Uh oh!
There was an error while loading. Please reload this page.
feat(sdk): on shutdown run all close calls in parallel - #85
Conversation
| } | ||
| var errs []error | ||
| for i := 0; i < len(plugins); i++ { | ||
| if err := <-errCh; err != nil { |
There was a problem hiding this comment.
so this is blocking, there are no guarantees that pl.Close() will return instantly, making this still O(n) since it waits for each Close() to write to the channel.
There was a problem hiding this comment.
also, O(n) means we are looping over all plugins, we will always need to loop over it, regardless of if it returns instantly or not.
There was a problem hiding this comment.
Right, I'll update my comment to T(1), the time complexity is constant now, that's what this PR is about
Uh oh!
There was an error while loading. Please reload this page.
| type mockRuntime struct { | ||
| name string | ||
| done chan struct{} | ||
| wait chan struct{} | ||
| } | ||
| func (m *mockRuntime) GetSecret(context.Context, secrets.Request) (secrets.Envelope, error) { | ||
| return secrets.Envelope{}, nil | ||
| } | ||
| func (m *mockRuntime) Close() error { | ||
| close(m.wait) | ||
| <-m.done | ||
| return fmt.Errorf("%s closed", m.name) | ||
| } | ||
| func (m *mockRuntime) Data() pluginData { | ||
| return pluginData{} | ||
| } | ||
| func Test_parallelStop(t *testing.T) { | ||
| r1 := &mockRuntime{name: "mock1", done: make(chan struct{}), wait: make(chan struct{})} | ||
| r2 := &mockRuntime{name: "mock2", done: make(chan struct{}), wait: make(chan struct{})} | ||
| r3 := &mockRuntime{name: "mock3", done: make(chan struct{}), wait: make(chan struct{})} | ||
| in := []runtime{r1, r2, r3} | ||
| stopErr := make(chan error) | ||
| go func() { | ||
| stopErr <- parallelStop(in) | ||
| }() | ||
| <-r3.wait | ||
| close(r3.done) | ||
| <-r1.wait | ||
| close(r1.done) | ||
| <-r2.wait | ||
| close(r2.done) | ||
| err := <-stopErr | ||
| assert.ErrorContains(t, err, "mock3 closed\nmock1 closed\nmock2 closed") | ||
| } |
There was a problem hiding this comment.
This test is racy. Unfortunately I don't see any other way to test this other than testing for what we actually want but with very extreme values: That the close time doesn't accumulate linearly with the number of plugins but approximately is capped to t_max.
| return pluginData{} | ||
| } | ||
| func Test_parallelStop(t *testing.T) { |
There was a problem hiding this comment.
I'm not aware of any other way other than actually using timeouts to make this test not racy. However, if we use extreme values (if the logic were broken and the sleeps would accumulate, it would build up to minutes easily), we can still make this a reliable test.
Uh oh!
There was an error while loading. Please reload this page.
| } | ||
| func (m *mockRuntime) Close() error { | ||
| time.Sleep(10 * time.Millisecond) |
There was a problem hiding this comment.
perhaps choose a random number between 1-10ms for the timeout, making each Close variable (which would be more realistic)
There was a problem hiding this comment.
I'd want this to reliably blow if something is broken
Uh oh!
There was an error while loading. Please reload this page.
I'm building up the engine logic and this is some tiny functionality that run fast shutdown.