2017-02-15 3 views
0

Nous utilisons Spring Boot pour développer nos services. Nous avons choisi de le faire d'une manière async et nous sommes confrontés au problème suivant: Nous avons l'aspect suivant au-dessus de toutes nos ressources de repos async:Spring Boot DeferredResult L'aspect rend le CPU très haut

import org.aspectj.lang.ProceedingJoinPoint; 
import org.aspectj.lang.annotation.Around; 
import org.aspectj.lang.annotation.Aspect; 
import org.aspectj.lang.reflect.MethodSignature; 
import org.slf4j.Logger; 
import org.slf4j.LoggerFactory; 
import org.springframework.aop.aspectj.MethodInvocationProceedingJoinPoint; 
import org.springframework.beans.factory.annotation.Autowired; 
import org.springframework.http.HttpStatus; 
import org.springframework.http.ResponseEntity; 
import org.springframework.stereotype.Component; 
import org.springframework.web.context.request.async.AsyncRequestTimeoutException; 
import org.springframework.web.context.request.async.DeferredResult; 


@Aspect 
@Component 
public class TerminatedUsersAspect { 

    private static final Logger LOGGER = LoggerFactory.getLogger("[TERMINATED_USERS]"); 
    public static final ThreadLocal<String> ID_LOCAL = new ThreadLocal<>(); 

    @Autowired 
    private UserRepository userRepo; 

    @Autowired 
    private UserService userService; 

    @Autowired 
    private ExecutorService executorService; 

    @SuppressWarnings("unchecked") 
    @Around("within(com.test..*) && @annotation(authorization)") 
    public Object checkForId(ProceedingJoinPoint joinPoint, Authorization authorization) throws Throwable { 

     final MethodInvocationProceedingJoinPoint mJoinPoint = (MethodInvocationProceedingJoinPoint) joinPoint; 
     final MethodSignature signature = (MethodSignature) mJoinPoint.getSignature(); 
     final DeferredResult<Object> ret = new DeferredResult<>(60000L); 
     final String id = ID_LOCAL.get(); 

     if (signature.getReturnType().isAssignableFrom(DeferredResult.class) && (id != null)) { 

      userRepo.getAccountStatus(id).thenAcceptAsync(status -> { 
       boolean accountValid = userService.isAccountValid(status, true); 

       if (!accountValid) { 
        LOGGER.debug("AccountId: {} is not valid. Rejecting with 403", id); 
        ret.setErrorResult(new ResponseEntity<String>("Invalid account.", HttpStatus.FORBIDDEN)); 
        return; 
       } 

       try { 

        final DeferredResult<Object> funcRet = (DeferredResult<Object>) joinPoint.proceed(); 

        funcRet.setResultHandler(r -> { 
         ret.setResult(r); 
        }); 

        funcRet.onTimeout(() -> { 
         ret.setResult(new AsyncRequestTimeoutException()); 
        }); 

       } catch (Throwable e) { 
        ret.setErrorResult(ret); 
       } 

      }, executorService).exceptionally(ex -> { 
       ret.setErrorResult(ex); 
       return null; 
      }); 

      return ret; 
     } 

     return joinPoint.proceed(); 
    } 

} 

Notre serveur embarqué dans l'application est undertow. Le problème se pose à temps. Il semble que, après presque un jour à cause de cet aspect, les processeurs finissent par atteindre 100% de rouge. J'ai débogué le code et semble aller bien de mon point de vue, mais peut-être qu'il me manque quelque chose? Toutes les idées seraient les bienvenues. Merci, C.

Répondre

1

La première chose qui me frappe est que vous appelez joinPoint.proceed() deux fois dans votre code. Le premier lorsque vous le cast à DeferredResult et le second lorsque vous revenez de la méthode. Vous devriez le réécrire afin qu'il ne soit exécuté qu'une seule fois. Vous pouvez envisager d'utiliser l'annotation @Around avec une 'exécution' (https://docs.spring.io/spring/docs/current/spring-framework-reference/html/aop.html#aop-pointcuts-examples) au lieu de 'within' car elle vous permet de spécifier le type de retour. Dans ce cas, vous n'en aurez pas besoin si vous êtes basé sur le type de retour de la méthode. Cela dit, je ne sais pas si cela va résoudre votre problème. Il me manque un contexte et je ne sais pas ce que vous essayez d'accomplir. Peut-être existe-t-il une façon plus simple de le faire, car l'utilisation du résultat différé dans ce contexte est un peu étrange pour moi. D'ailleurs, qu'est-ce qui causait particulièrement cette utilisation de 100% des ressources? Y a-t-il des threads qui s'exécutent à l'infini ou des connexions HTTP qui pendaient?

+0

Je suis d'accord avec tout ce qui est dit ici. Vous semblez appliquer l'aspect trop largement, le grand «si» et toutes les choses coûteuses de réflexion ne sont pas nécessaires. Alors le 'procéder()' après le 'si 'ne serait pas nécessaire en premier lieu, mais si vous l'utilisez, mettez-le au moins dans une branche' else'. Je vais préparer un petit échantillon pour vous. – kriegaex

0

Que diriez-vous de ce petit refactoring? Il est de typesafe, n'a pas besoin de réflexion et n'appelle qu'une seule fois proceed().

De plus, je pensais que dans votre code ret.setErrorResult(ret) n'a pas beaucoup de sens et je l'ai remplacé par setErrorResult(e), le réglage de la Execption réelle que le résultat d'erreur. J'espere aussi que vous me pardonnerez de renommer quelques variables.

Cela m'a aidé à mieux comprendre ce qui se passait. Je ne suis toujours pas sûr à 100% car je ne sais rien sur Spring (mais beaucoup sur AspectJ).

package de.scrum_master.aspect; 

import java.util.concurrent.ExecutorService; 

import org.aspectj.lang.ProceedingJoinPoint; 
import org.aspectj.lang.annotation.Around; 
import org.aspectj.lang.annotation.Aspect; 
import org.slf4j.Logger; 
import org.slf4j.LoggerFactory; 
import org.springframework.beans.factory.annotation.Autowired; 
import org.springframework.http.HttpStatus; 
import org.springframework.http.ResponseEntity; 
import org.springframework.stereotype.Component; 
import org.springframework.web.context.request.async.AsyncRequestTimeoutException; 
import org.springframework.web.context.request.async.DeferredResult; 

@Aspect 
@Component 
public class TerminatedUsersAspect { 

    private static final Logger LOGGER = LoggerFactory.getLogger("[TERMINATED_USERS]"); 
    public static final ThreadLocal<String> ID_LOCAL = new ThreadLocal<>(); 

    @Autowired private UserRepository userRepo; 
    @Autowired private UserService userService; 
    @Autowired private ExecutorService executorService; 

    @Around(
    "within(com.test..*) && @annotation(authorization) && " + 
    "execution(org.springframework.web.context.request.async.DeferredResult *(..))" 
) 
    public DeferredResult<Object> checkForId(ProceedingJoinPoint joinPoint, Authorization authorization) throws Throwable { 

    final DeferredResult<Object> aspectResult = new DeferredResult<>(60000L); 
    final String id = ID_LOCAL.get(); 

    userRepo 
     .getAccountStatus(id) 
     .thenAcceptAsync(status -> { 
      boolean accountValid = userService.isAccountValid(status, true); 
      if (!accountValid) { 
      LOGGER.debug("AccountId: {} is not valid. Rejecting with 403", id); 
      aspectResult.setErrorResult(new ResponseEntity<String>("Invalid account.", HttpStatus.FORBIDDEN)); 
      return; 
      } 
      try { 
      @SuppressWarnings("unchecked") 
      final DeferredResult<Object> originalResult = (DeferredResult<Object>) joinPoint.proceed(); 
      originalResult.setResultHandler(r -> { aspectResult.setResult(r); }); 
      originalResult.onTimeout(() -> { aspectResult.setResult(new AsyncRequestTimeoutException()); }); 
      } catch (Throwable e) { 
      aspectResult.setErrorResult(e); 
      } 
     }, 
     executorService 
    ) 
     .exceptionally(ex -> { 
      aspectResult.setErrorResult(ex); 
      return null; 
     } 
    ); 

    return aspectResult; 
    } 
}