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

Ftr: Hystrix filter for circuit break and service downgrade #133

Merged
merged 38 commits into from
Aug 26, 2019

Conversation

YGrylls
Copy link
Contributor

@YGrylls YGrylls commented Jul 22, 2019

What this PR does:

Add an implement of filter for circuit break, based on hystrix-go, see filter/impl/hystrix_filter.go
The config of hystrix is exposed to users in the consumer config. Thus a new field FilterConf is added, just like the protocol conf existed.
The service downgrade can also be customized by users by defining extra filters that handles the error returned from hystrix filter, an example is offered
This filter is designed for both client and server end, now only client works because temporarily filters in server end can not get the return information from callService, see protocol/dubbo/listener OnMessage(). Once they are integrated in the future, the filter can be used in server end just like in client end

Special notes for your reviewer:

Unit test is done, see filter/impl/hystrix_filter_test.go

Does this PR introduce a user-facing change?:

  • To use the hystrix filter, add "hystrix_consumer" to the filter of references part in the consumer config
  • Config the hystrix in the filter_conf part of the consumer config file as the example does, the config can be set at either service or method level
  • An error whitelist can be defined in the config to decide what error will trigger hystrix and what will not
  • You can define fallback logic by defining customized filter, see how examples/hystrixfilter/dubbo/with-hystrix-go-client/app/example_fallback_filter.go does. Remember to add your fallback filter JUST BEFORE the hystrix filter

@hxmhlt
Copy link
Contributor

hxmhlt commented Jul 22, 2019

pls resolve the conflicting files

YGrylls added 8 commits July 22, 2019 13:42
For implement of hystrix filter depends on config package to get user config, it causes import cycle in test for protocol filter wrapper.
Now the test depends on filter interface instead of filter implement, so that the cycle is solved.
@codecov-io
Copy link

codecov-io commented Jul 23, 2019

Codecov Report

Merging #133 into develop will increase coverage by 1.01%.
The diff coverage is 68.64%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #133      +/-   ##
===========================================
+ Coverage    66.85%   67.86%   +1.01%     
===========================================
  Files           93       94       +1     
  Lines         5729     5847     +118     
===========================================
+ Hits          3830     3968     +138     
+ Misses        1491     1466      -25     
- Partials       408      413       +5
Impacted Files Coverage Δ
config/consumer_config.go 61.7% <ø> (ø) ⬆️
config/provider_config.go 61.76% <ø> (ø) ⬆️
filter/impl/hystrix_filter.go 68.64% <68.64%> (ø)
protocol/dubbo/readwriter.go 74.35% <0%> (+2.56%) ⬆️
protocol/dubbo/codec.go 82.35% <0%> (+8.82%) ⬆️
protocol/dubbo/listener.go 64.81% <0%> (+10.64%) ⬆️
protocol/dubbo/pool.go 75% <0%> (+15.76%) ⬆️

Continue to review full report at Codecov.

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

YGrylls added 3 commits July 24, 2019 18:20
…rror whitelist(omit)

TODO: Make downgrade a standalone filter, so it can offer more freedom for users and avoid extensions on extensions
TODO: Add a new example
fix the problem that all methods under one service share the regexp rules
filter/impl/hystrix_filter_test.go Outdated Show resolved Hide resolved
filter/impl/hystrix_filter_test.go Outdated Show resolved Hide resolved
filter/impl/hystrix_filter.go Outdated Show resolved Hide resolved
protocol/protocolwrapper/protocol_filter_wrapper_test.go Outdated Show resolved Hide resolved
@YGrylls YGrylls changed the title Hystrix filter for circuit break and service downgrade Ftr: Hystrix filter for circuit break and service downgrade Aug 12, 2019
@hxmhlt hxmhlt merged commit 14436e9 into apache:develop Aug 26, 2019
@YGrylls YGrylls deleted the hystrix_filter branch August 26, 2020 01:24
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.

4 participants