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

support setting default rules for resources #966

Closed
wants to merge 36 commits into from
Closed

support setting default rules for resources #966

wants to merge 36 commits into from

Conversation

linlinisme
Copy link
Collaborator

Does this pull request fix one issue?

Fixes #66

Describe how you did it

I adopt another design:
the default rules can be created and added in the resource pre-source init while application first time access resource.
I put this op in the old lookProcessChain method, now it rename to lookProcessChainAndInitDefaultRules.
The reason I do this as follows:

1. resource corresponding default rules can be created one time and only one time.
2. it's thread safe, if we do it at other place we may need extract syn op.
3. setting resource's default rules can be treated as a part of init method.
4. it's created on demand.

Describe how to verify it

run test cases.

Special notes for reviews

Lin.Liang and others added 30 commits May 9, 2019 14:54
using the LongAdder rather than AtomicInteger to Provides better performance
@sczyh30 sczyh30 added kind/feature Category issues or prs related to feature request. to-review To review labels Aug 6, 2019
@linlinisme linlinisme closed this Mar 6, 2020
@pigercc
Copy link

pigercc commented Jun 2, 2020

if (SentinelConfig.isGlobalRuleOpen()) {
                for (Map.Entry<String, Set<DegradeRule>> entry : defaultDegradeRules.entrySet()) {
                    if (degradeRules.containsKey(entry.getKey())) {
                        defaultDegradeRules.remove(entry.getKey());
                    } else {
                        degradeRules.put(entry.getKey(), entry.getValue());
                    }
                }
            }

thanks for the code,it is helpful when i also need the global rule,but i think the code has a bug

the right expectation is:
if I set degrade rule, use mine
else use the global rule

but actual performance:

  1. lookProcessChainAndInitDefaultRules init
 setRulesForResource(resource, degradeConf.get(resource));
defaultDegradeRules.put(resource, degradeConf.get(resource));

2 I set my degrade rule, is work
3 I remove my degrade rule,but now cannot apply the global rule

when remove happend,nacos handler will call
public void configUpdate(List<DegradeRule> conf)
and conf of course not cantains the global rule

and in the for code,defaultDegradeRules has remove resource

 for (Map.Entry<String, Set<DegradeRule>> entry : defaultDegradeRules.entrySet())

so degradeRules cannot add the global rule back

degradeRules.put(entry.getKey(), entry.getValue());

@sczyh30 sczyh30 removed the to-review To review label Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Category issues or prs related to feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Support setting the universal default rule for all resources
3 participants