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

issue: Component scan two times for @Service #7477

Closed
wants to merge 2 commits into from

Conversation

@zhangjiezhang
Copy link
Contributor

@zhangjiezhang zhangjiezhang commented Mar 30, 2021

Add cache to solve this issue

@kylixs
Copy link
Contributor

@kylixs kylixs commented Apr 1, 2021

Repeated scan should be a problem in the implementation logic. There are two ways to register a scanner, one is to annotate @DubboComponentScan/@EnableDubbo, and the other is to define the configuration: dubbo.scan.base-packages .
A better solution is to register a single scanner (prefer to ServiceClassPostProcessor), then collect all the package names to be scanned from the annotations and configuration, filter the duplicate packages and sub-packages, and then scan.

@chickenlj
Copy link
Contributor

@chickenlj chickenlj commented Jun 10, 2021

Please check @kylixs 's comments.

@zhangjiezhang
Copy link
Contributor Author

@zhangjiezhang zhangjiezhang commented Jun 11, 2021

Please check @kylixs 's comments.

My project uses ServiceClassPostProcessor and only scans once for Component's(DubboService, org.apache.dubbo.config.annotation.Service, com.alibaba.dubbo.config.annotation.Service).

org.apache.dubbo.config.spring.beans.factory.annotation.ServiceClassPostProcessor is used to run org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider.findCandidateComponents(String basePackage) twice:
1.scanner.scan(packageToScan) -> scanner.doScan(basePackages) -> scanner.findCandidateComponents(basePackage): add DubboService lost by AnnotationConfigApplicationContext scan to spring container.
2.ServiceClassPostProcessor.findServiceBeanDefinitionHolders(scanner, packageToScan, registry, beanNameGenerator) -> scanner.findCandidateComponents(packageToScan): get All DubboService.

@kylixs
Copy link
Contributor

@kylixs kylixs commented Jun 11, 2021

Duplicated scanning is resolved in Dubbo 3 :

private void scanServiceBeans(Set<String> packagesToScan, BeanDefinitionRegistry registry) {

@zhangjiezhang
Copy link
Contributor Author

@zhangjiezhang zhangjiezhang commented Jun 11, 2021

Duplicated scanning is resolved in Dubbo 3 :

A0AF1E21-F218-4621-B1FA-768EF018048F
Still scan twice!!!
The scan results of the first step should be cached for the second step of query。

@kylixs
Copy link
Contributor

@kylixs kylixs commented Jun 11, 2021

First, avoid scan duplicated packages or sub-package:

  // avoid duplicated scans
  if (servicePackagesHolder.isPackageScanned(packageToScan)) {
      if (logger.isInfoEnabled()) {
          logger.info("Ignore package who has already bean scanned: " + packageToScan);
      }
      continue;
  }

Then, exclude scanned classes before generate BeanDefinitions:

ScanExcludeFilter scanExcludeFilter = new ScanExcludeFilter();        
scanner.addExcludeFilter(scanExcludeFilter);
private class ScanExcludeFilter implements TypeFilter {

    private int excludedCount;

    @Override
    public boolean match(MetadataReader metadataReader, MetadataReaderFactory metadataReaderFactory) throws IOException {
        String className = metadataReader.getClassMetadata().getClassName();
        boolean excluded = servicePackagesHolder.isClassScanned(className);
        if (excluded) {
            excludedCount ++;
        }
        return excluded;
    }

    public int getExcludedCount() {
        return excludedCount;
    }
}

@zhangjiezhang
Copy link
Contributor Author

@zhangjiezhang zhangjiezhang commented Jun 11, 2021

Then, exclude scanned classes before generate BeanDefinitions:

98DAAB80-2CB8-46D7-AE93-8B7EDE9AEB50
1.the first scan: scanner.scan(packageToScan)
2.the second scan: findServiceBeanDefinitionHolders(scanner, packageToScan, registry, beanNameGenerator)
You just add classfile to servicePackagesHolder after the second scan, and you cannot avoid called scanner.findCandidateComponents(String basePackage) twice for same class.
Because the classfile scanned for the first time has not been added to servicePackagesHolder.

@kylixs
Copy link
Contributor

@kylixs kylixs commented Jun 11, 2021

1.the first scan: scanner.scan(packageToScan)
2.the second scan: findServiceBeanDefinitionHolders(scanner, packageToScan, registry, beanNameGenerator)

Oh, you're right, I miss it.

The first scan is to register the bean definition of the service implementation class, and the second scan is to register the bean definition of the dubbo ServiceBean later. So, I think the second scan is unnecessary, just use the results of the first scan directly.

Some thing like this:

private void scanServiceBeans(Set<String> packagesToScan, BeanDefinitionRegistry registry) {
   ...
    for (String packageToScan : packagesToScan) {

        // avoid duplicated scans
        if (servicePackagesHolder.isPackageScanned(packageToScan)) {
            if (logger.isInfoEnabled()) {
                logger.info("Ignore package who has already bean scanned: " + packageToScan);
            }
            continue;
        }

        // Scan and registers as Spring @Service Bean first
        Set<BeanDefinitionHolder> beanDefinitionHolders = scanner.doScan(packageToScan);

        // Second, register bean definition of dubbo ServiceBean for each service
        if (!CollectionUtils.isEmpty(beanDefinitionHolders)) {
            ...
            for (BeanDefinitionHolder beanDefinitionHolder : beanDefinitionHolders) {
                processScannedBeanDefinition(beanDefinitionHolder, registry, scanner);
                servicePackagesHolder.addScannedClass(beanDefinitionHolder.getBeanDefinition().getBeanClassName());
            }
        } else {
            if (logger.isWarnEnabled()) {
                logger.warn("No class annotated by Dubbo @Service was found under package ["
                        + packageToScan + "], ignore re-scanned classes: " + scanExcludeFilter.getExcludedCount());
            }
        }
        servicePackagesHolder.addScannedPackage(packageToScan);
    }
}

However, if the @Service annotation of Spring is scanned first, and the @DubboService annotation of dubbo is scanned secondly, the bean of the service implementation class has already been registered, it will be ignored.
So it is necessary to re-implement the logic of doScan, regardless of whether the beanDefinition already exists, or create a bean definition to register the dubbo ServiceBean.

public Set<BeanDefinitionHolder> doScan(BeanDefinitionRegistry registry, BeanNameGenerator beanNameGenerator, String basePackage) {
// Registers @Service Bean first
super.doScan(basePackage);
Set<BeanDefinition> beanDefinitions = findCandidateComponents(basePackage);
Copy link
Contributor

@kylixs kylixs Jun 11, 2021

Choose a reason for hiding this comment

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

super.doScan(basePackage): will do first scanning
findCandidateComponents() : will do second scanning
This did not reduce scanning.

Copy link
Contributor Author

@zhangjiezhang zhangjiezhang Jun 11, 2021

Choose a reason for hiding this comment

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

super.doScan(basePackage): Call method findCandidateComponents() for first time run, this method will cache scan result.
findCandidateComponents(basePackage): this method run for the second time and will use cached results instead of scanning.
So this reduce scanning for the second time.

Copy link
Contributor Author

@zhangjiezhang zhangjiezhang Jun 11, 2021

Choose a reason for hiding this comment

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

I rewrite method findCandidateComponents for DubboClassPathBeanDefinitionScanner.

Copy link
Contributor

@kylixs kylixs Jun 11, 2021

Choose a reason for hiding this comment

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

I think just cache Set<BeanDefinitionHolder> in findCandidateComponents method is enough, do not need add new doScan method

Copy link
Contributor Author

@zhangjiezhang zhangjiezhang left a comment

Solve the cause of the problem

public Set<BeanDefinitionHolder> doScan(BeanDefinitionRegistry registry, BeanNameGenerator beanNameGenerator, String basePackage) {
// Registers @Service Bean first
super.doScan(basePackage);
Set<BeanDefinition> beanDefinitions = findCandidateComponents(basePackage);
Copy link
Contributor Author

@zhangjiezhang zhangjiezhang Jun 11, 2021

Choose a reason for hiding this comment

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

I rewrite method findCandidateComponents for DubboClassPathBeanDefinitionScanner.

@kylixs
Copy link
Contributor

@kylixs kylixs commented Jun 11, 2021

Another way is to cache the beanName and BeanDefinition in the postProcessBeanDefinition method, so that there is no need to regenerate the beanName, which is more reliable.

public class DubboClassPathBeanDefinitionScanner extends ClassPathBeanDefinitionScanner {

    private Set<BeanDefinitionHolder> beanDefinitionHolders = new LinkedHashSet<>();
    ....

    @Override
    protected void postProcessBeanDefinition(AbstractBeanDefinition beanDefinition, String beanName) {
        beanDefinitionHolders.add(new BeanDefinitionHolder(beanDefinition, beanName));
        super.postProcessBeanDefinition(beanDefinition, beanName);
    }

    public Set<BeanDefinitionHolder> getBeanDefinitionHolders() {
        return beanDefinitionHolders;
    }
}

// ServiceAnnotationPostProcessor

    private void scanServiceBeans(Set<String> packagesToScan, BeanDefinitionRegistry registry) {
      ...
        for (String packageToScan : packagesToScan) {
            ...
            // Scan and registers as Spring @Service Bean first
            // if the service class is already scanned and registered by @ComponentScan, the bean definitions will be ignored.
            scanner.scan(packageToScan);

            Set<BeanDefinitionHolder> beanDefinitionHolders = scanner.getBeanDefinitionHolders();

            // Second, register bean definition of dubbo ServiceBean for each service
            if (!CollectionUtils.isEmpty(beanDefinitionHolders)) {
            ....
      ....
}

@zhangjiezhang
Copy link
Contributor Author

@zhangjiezhang zhangjiezhang commented Jun 11, 2021

Another way is to cache the beanName and BeanDefinition in the postProcessBeanDefinition method, so that there is no need to regenerate the beanName, which is more reliable.

public class DubboClassPathBeanDefinitionScanner extends ClassPathBeanDefinitionScanner {

    private Set<BeanDefinitionHolder> beanDefinitionHolders = new LinkedHashSet<>();
    ....

    @Override
    protected void postProcessBeanDefinition(AbstractBeanDefinition beanDefinition, String beanName) {
        beanDefinitionHolders.add(new BeanDefinitionHolder(beanDefinition, beanName));
        super.postProcessBeanDefinition(beanDefinition, beanName);
    }

    public Set<BeanDefinitionHolder> getBeanDefinitionHolders() {
        return beanDefinitionHolders;
    }
}

// ServiceAnnotationPostProcessor

    private void scanServiceBeans(Set<String> packagesToScan, BeanDefinitionRegistry registry) {
      ...
        for (String packageToScan : packagesToScan) {
            ...
            // Scan and registers as Spring @Service Bean first
            // if the service class is already scanned and registered by @ComponentScan, the bean definitions will be ignored.
            scanner.scan(packageToScan);

            Set<BeanDefinitionHolder> beanDefinitionHolders = scanner.getBeanDefinitionHolders();

            // Second, register bean definition of dubbo ServiceBean for each service
            if (!CollectionUtils.isEmpty(beanDefinitionHolders)) {
            ....
      ....
}

Yes, you're right.👍
Either way is OK.

@kylixs
Copy link
Contributor

@kylixs kylixs commented Jun 15, 2021

@zhangjiezhang Please improve it and resubmit pr.

@zhangjiezhang
Copy link
Contributor Author

@zhangjiezhang zhangjiezhang commented Jun 15, 2021

@zhangjiezhang Please improve it and resubmit pr.

Which branch to modify?

zhangjiezhang pushed a commit to zhangjiezhang/dubbo that referenced this issue Jun 15, 2021
@kylixs
Copy link
Contributor

@kylixs kylixs commented Jun 15, 2021

Create two pull requests separately based on master and 3.0 branches, please use concise code whenever possible.

zhangjiezhang pushed a commit to zhangjiezhang/dubbo that referenced this issue Jun 16, 2021
AlbumenJ pushed a commit that referenced this issue Jun 21, 2021
* issue: Component scan two times for @service

* add cache for scan result. (#7477)

* remove redundant comments.

Co-authored-by: zhang.jie <zhangjie@pascall.xyz>
Co-authored-by: zhang.jie <zhangjie@rivamed.cn>
AlbumenJ pushed a commit that referenced this issue Jun 21, 2021
* add cache for scan result. (#7477)

* remove unused method and delete Unused import. (#8049)

* delete checkCandidate method.

* Please remove redundant comments.

Co-authored-by: zhang.jie <zhangjie@rivamed.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants