Optimize performance of '/apps/{appId}/envs/{env}/clusters/{clusterName}/namespaces' interface queries

[klboke]

What's the purpose of this PR

• As shown in the figure below, launching a 'findNamespaces' query will process each 'Namespace' in turn in the loop body, and when the 'Namespace ' is very large, e.g. up to 40, the total time taken is exaggerated to 7-8 seconds because of single-threaded serial execution.

• Changing to multi-threaded processing of each 'Namespace' will have a very significant improvement on the 'findNamespaces' interface latency

Timing diagram from @Anilople

[codecov-commenter]

Codecov Report

Merging #4473 (c9de54d) into master (aaa3561) will increase coverage by 0.05%.
The diff coverage is 95.23%.

@@             Coverage Diff              @@
##             master    #4473      +/-   ##
============================================
+ Coverage     53.56%   53.62%   +0.05%     
- Complexity     2698     2700       +2     
============================================
  Files           489      489              
  Lines         15281    15294      +13     
  Branches       1587     1588       +1     
============================================
+ Hits           8186     8202      +16     
+ Misses         6540     6537       -3     
  Partials        555      555              

Impacted Files	Coverage Δ
...mework/apollo/portal/service/NamespaceService.java	71.26% <95.23%> (+4.18%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

[nobodyiam]

The original logic will throw the actual exception so that the user knows what's wrong. Is it better to preserve that logic? For example, we could use future.get to retrieve the exception occurred.

    List<NamespaceBO> namespaceBOs = Collections.synchronizedList(new LinkedList<>());
    List<Future<?>> futures = new LinkedList<>();
    for (NamespaceDTO namespace : namespaces) {
      futures.add(executorService.submit(() -> {
        NamespaceBO namespaceBO;
        try {
          namespaceBO = transformNamespace2BO(env, namespace);
          namespaceBOs.add(namespaceBO);
        } catch (Exception e) {
          LOGGER.error("parse namespace error. app id:{}, env:{}, clusterName:{}, namespace:{}",
              appId, env, clusterName, namespace.getNamespaceName(), e);
          throw e;
        }
      }));
    }

    futures.forEach(future -> {
      try {
        future.get();
      } catch (ExecutionException | InterruptedException e) {
        Throwable exception = e instanceof ExecutionException ? e.getCause() : e;
        if (exception instanceof RuntimeException) {
          throw (RuntimeException) exception;
        } else {
          throw new RuntimeException(exception);
        }
      }
    });

[klboke]

LOGGER.error("parse namespace error. app id:{}, env:{}, clusterName:{}, namespace:{}",
              appId, env, clusterName, namespace.getNamespaceName(), e);

The original exception stack information will be recorded here

[nobodyiam]

I understand they are recorded in the server logs. What I propose is to throw the detailed exception message to the UI page so that the operating users could understand what's wrong, which is the original behavior.
The current error message Parse namespaces error, expected: %s, but actual: %s, cannot get those namespaces: %s could show the operating users some error details, but I have a concern that it might not be very clear.

[klboke]

I probably see what you mean, and the main points I took into account when making the changes here were.

• I am not very clear about the internal logic will report what possible exceptions, but when I simulated an exception, as a direct user it is difficult to do anything based on this exception, may be my simulation and the real difference
• the original will only encounter a Namespace problem to report an exception, now all the problems will be reported at once, will it be better?

[nobodyiam]

I am not very clear about the internal logic will report what possible exceptions, but when I simulated an exception, as a direct user it is difficult to do anything based on this exception, may be my simulation and the real difference

The exceptions would be either timeout exceptions occurred in the portal side, or database access exceptions occurred in the admin service side. I suppose there is not much to do for a direct user, except that the user could report that to the admin users.
So my original idea was that we keep the logic the same as before - throw the actual exception and display it in the UI side, but I think the current logic is also ok.

the original will only encounter a Namespace problem to report an exception, now all the problems will be reported at once, will it be better?

That I suppose is the same as the user can't really handle the errors, no matter it contains one error or multiple errors :-)

[nobodyiam]

LGTM