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

[BUG] ProcessorSlotChain may get wrong one #3311

Open
Daydreamer-ia opened this issue Jan 20, 2024 · 0 comments
Open

[BUG] ProcessorSlotChain may get wrong one #3311

Daydreamer-ia opened this issue Jan 20, 2024 · 0 comments

Comments

@Daydreamer-ia
Copy link

Daydreamer-ia commented Jan 20, 2024

Issue Description

When I was reading the source code of Sentinel, I found that the ProcessorSlotChain obtained varies for different resources. I found that ResourceWrapper overrides the equals and hashcode to get unique chain here.

    // ResourceWrapper 
    @Override
    public int hashCode() {
        return getName().hashCode();
    }

    @Override
    public boolean equals(Object obj) {
        if (obj instanceof ResourceWrapper) {
            ResourceWrapper rw = (ResourceWrapper)obj;
            return rw.getName().equals(getName());
        }
        return false;
    }

    // CtSph
    ProcessorSlot<Object> lookProcessChain(ResourceWrapper resourceWrapper) {
        ProcessorSlotChain chain = chainMap.get(resourceWrapper);
        if (chain == null) {
            synchronized (LOCK) {
                chain = chainMap.get(resourceWrapper);
                if (chain == null) {
                    // Entry size limit.
                    if (chainMap.size() >= Constants.MAX_SLOT_CHAIN_SIZE) {
                        return null;
                    }

                    chain = SlotChainProvider.newSlotChain();
                    Map<ResourceWrapper, ProcessorSlotChain> newMap = new HashMap<ResourceWrapper, ProcessorSlotChain>(
                        chainMap.size() + 1);
                    newMap.putAll(chainMap);
                    newMap.put(resourceWrapper, chain);
                    chainMap = newMap;
                }
            }
        }
        return chain;
    }

As you see, there will use the same chain if resources have the same name, even if they are different resource type. But as user, we may use the same name for different resource by accident. Is there any wrong happen with sentinel because of this case?
The define in ClusterNode considers the name and type. Why not consider as the same at ResourceWrapper?

public class ClusterNode extends StatisticNode {

    private final String name;
    private final int resourceType;
}

I'm not sure if my idea is right. If anything wrong, please correct me.

Describe what happened

Describe what you expected to happen

Maybe we need to consider resource type for resourceWrapper when geting chain.

    // ResourceWrapper 
    @Override
    public int hashCode() {
        return (getName() + getResourceType()).hashCode();
    }

    @Override
    public boolean equals(Object obj) {
        if (obj instanceof ResourceWrapper) {
            ResourceWrapper rw = (ResourceWrapper)obj;
            return rw.getName().equals(getName()) && rw.getResourceType() == getResourceType();
        }
        return false;
    }

How to reproduce it (as minimally and precisely as possible)

Tell us your environment

Anything else we need to know?

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

No branches or pull requests

1 participant