Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor breaker to link call to result #3

Merged
merged 7 commits into from
Oct 11, 2023

Conversation

costela
Copy link
Member

@costela costela commented Oct 8, 2023

This PR lays the groundwork e.g. for an optional prometheus wrapper, linking each execution to its result.

This refactor is a breaking change. The API now looks like:

h := hoglet.NewCircuit(
    func(ctx context.Context, foo Foo) (Bar, error) {
        // do stuff
        return Bar{}, nil
    },
    hoglet.NewSlidingWindowBreaker(10, 0.1),
    hoglet.WithFailureCondition(hoglet.IgnoreContextCancelation),
)
f, err := h.Call(context.Background(), Foo{/* ... */})
//...

Benchmarks against latest hystrix:

cpu: 12th Gen Intel(R) Core(TM) i7-1250U
BenchmarkHoglet
BenchmarkHoglet-12     	30580755	       343.3 ns/op	     233 B/op	       6 allocs/op
BenchmarkHystrix
BenchmarkHystrix-12    	13177122	       970.2 ns/op	    1217 B/op	      23 allocs/op
benchmark code

package bench_test

import (
	"context"
	"testing"

	"github.com/exaring/hoglet"
	"github.com/exaring/hystrix-go/hystrix"
)

func BenchmarkHoglet(b *testing.B) {
	circuit := hoglet.NewCircuit(
		func(context.Context, any) (any, error) { return nil, nil },
		hoglet.NewEWMABreaker(10, 0.9),
	)

	ctx := context.Background()

	b.ReportAllocs()
	b.ResetTimer()

	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			_, _ = circuit.Call(ctx, nil)
		}
	})
}

func BenchmarkHystrix(b *testing.B) {
	cmdName := "foo"

	hystrix.ConfigureCommand(cmdName, hystrix.CommandConfig{
		Timeout:                250,
		MaxConcurrentRequests:  0, // unlimited; otherwise we skew the comparison
		ErrorPercentThreshold:  10,
		RequestVolumeThreshold: 20,
		SleepWindow:            2000,
	})

	ctx := context.Background()

	b.ReportAllocs()
	b.ResetTimer()

	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			_ = hystrix.DoC(ctx, "foo", func(context.Context) (error) { return nil }, nil)
		}
	})
}

@costela costela marked this pull request as ready for review October 10, 2023 16:23
breaker.go Outdated Show resolved Hide resolved
hoglet.go Show resolved Hide resolved
Co-authored-by: Alexander Knipping <[email protected]>
@costela costela requested a review from obitech October 11, 2023 08:06
Copy link
Member

@obitech obitech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

breaker.go Outdated Show resolved Hide resolved
hoglet.go Outdated Show resolved Hide resolved
This should make the code less surprising: calling a nil function
variable panics, so calling a zero value circuit is analogous to that.
@costela costela force-pushed the refactor-breaker-to-link-call-to-result branch from bf3909f to 135c82b Compare October 11, 2023 16:06
@costela costela merged commit f1e1e09 into main Oct 11, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants