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

add runnable/callable class in async invoke method #1327

Merged
merged 1 commit into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -111,19 +112,8 @@
if (isParallelCheck()) {
CountDownLatch countDownLatch = new CountDownLatch(readinessHealthCheckers.size());
AtomicBoolean parallelResult = new AtomicBoolean(true);
readinessHealthCheckers.forEach((String key, HealthChecker value) -> healthCheckExecutor.execute(() -> {
try {
if (!doHealthCheck(key, value, false, healthMap, true, false)) {
parallelResult.set(false);
}
} catch (Throwable t) {
parallelResult.set(false);
logger.error(ErrorCode.convert("01-22004"), t);
healthMap.put(key, new Health.Builder().withException(t).status(Status.DOWN).build());
} finally {
countDownLatch.countDown();
}
}));
readinessHealthCheckers.forEach((String key, HealthChecker value) -> healthCheckExecutor.execute(
new AsyncHealthCheckRunnable(key, value, healthMap, parallelResult, countDownLatch)));
Comment on lines +115 to +116
Copy link

Choose a reason for hiding this comment

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

The implementation of parallel health checks using AsyncHealthCheckRunnable is effective. Ensure that the timeout handling is robust and consider logging more details about the timeout event.

- healthMap.put("parallelCheck", new Health.Builder().withDetail("timeout", getParallelCheckTimeout()).status(Status.UNKNOWN).build());
+ logger.warn("Parallel health check timed out after {}ms.", getParallelCheckTimeout());
+ healthMap.put("parallelCheck", new Health.Builder().withDetail("timeout", getParallelCheckTimeout()).status(Status.UNKNOWN).build());

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
readinessHealthCheckers.forEach((String key, HealthChecker value) -> healthCheckExecutor.execute(
new AsyncHealthCheckRunnable(key, value, healthMap, parallelResult, countDownLatch)));
readinessHealthCheckers.forEach((String key, HealthChecker value) -> healthCheckExecutor.execute(
new AsyncHealthCheckRunnable(key, value, healthMap, parallelResult, countDownLatch)));
logger.warn("Parallel health check timed out after {}ms.", getParallelCheckTimeout());
healthMap.put("parallelCheck", new Health.Builder().withDetail("timeout", getParallelCheckTimeout()).status(Status.UNKNOWN).build());

boolean finished = false;
try {
finished = countDownLatch.await(getParallelCheckTimeout(), TimeUnit.MILLISECONDS);
Expand Down Expand Up @@ -160,7 +150,7 @@
* @return health check passes or not
*/
private boolean doHealthCheck(String beanId, HealthChecker healthChecker, boolean isRetry,
Map<String, Health> healthMap, boolean isReadiness, boolean wait) {
Map<String, Health> healthMap, boolean isReadiness, boolean wait) {
Assert.notNull(healthMap, "HealthMap must not be null");

Health health;
Expand All @@ -180,20 +170,23 @@
do {
try {
if (wait) {
Future<Health> future = healthCheckExecutor.submit(healthChecker::isHealthy);
Future<Health> future = healthCheckExecutor
.submit(new AsyncHealthCheckCallable(healthChecker));
health = future.get(timeout, TimeUnit.MILLISECONDS);
} else {
health = healthChecker.isHealthy();
}
} catch (TimeoutException e) {
logger.error(
} catch (TimeoutException e) {
logger
.error(
"Timeout occurred while doing HealthChecker[{}] {} check, the timeout value is: {}ms.",
beanId, checkType, timeout);
health = new Health.Builder().withException(e).withDetail("timeout", timeout).status(Status.UNKNOWN).build();
health = new Health.Builder().withException(e).withDetail("timeout", timeout)
.status(Status.UNKNOWN).build();
} catch (Throwable e) {
logger.error(String.format(
"Exception occurred while wait the result of HealthChecker[%s] %s check.",
beanId, checkType), e);
"Exception occurred while wait the result of HealthChecker[%s] %s check.",
beanId, checkType), e);
health = new Health.Builder().withException(e).status(Status.DOWN).build();
}
result = health.getStatus().equals(Status.UP);
Expand All @@ -208,9 +201,7 @@
retryCount += 1;
TimeUnit.MILLISECONDS.sleep(healthChecker.getRetryTimeInterval());
} catch (InterruptedException e) {
logger
.error(ErrorCode.convert("01-23002", retryCount, beanId,
checkType), e);
logger.error(ErrorCode.convert("01-23002", retryCount, beanId, checkType), e);

Check warning on line 204 in sofa-boot-project/sofa-boot-actuator/src/main/java/com/alipay/sofa/boot/actuator/health/HealthCheckerProcessor.java

View check run for this annotation

Codecov / codecov/patch

sofa-boot-project/sofa-boot-actuator/src/main/java/com/alipay/sofa/boot/actuator/health/HealthCheckerProcessor.java#L204

Added line #L204 was not covered by tests
}
}
} while (isRetry && retryCount < healthChecker.getRetryCount());
Expand All @@ -223,12 +214,12 @@
if (!result) {
if (healthChecker.isStrictCheck()) {
logger.error(ErrorCode.convert("01-23001", beanId, checkType, retryCount,
objectMapper.writeValueAsString(health.getDetails()),
healthChecker.isStrictCheck()));
objectMapper.writeValueAsString(health.getDetails()),
healthChecker.isStrictCheck()));
} else {
logger.warn(ErrorCode.convert("01-23001", beanId, checkType, retryCount,
objectMapper.writeValueAsString(health.getDetails()),
healthChecker.isStrictCheck()));
objectMapper.writeValueAsString(health.getDetails()),
healthChecker.isStrictCheck()));
}
}
} catch (JsonProcessingException ex) {
Expand Down Expand Up @@ -364,4 +355,55 @@
}

}

private class AsyncHealthCheckRunnable implements Runnable {
private final String key;
private final HealthChecker value;
private final Map<String, Health> healthMap;

private final AtomicBoolean parallelResult;

private final CountDownLatch countDownLatch;

public AsyncHealthCheckRunnable(String key, HealthChecker value,
Map<String, Health> healthMap,
AtomicBoolean parallelResult, CountDownLatch countDownLatch) {
this.key = key;
this.value = value;
this.healthMap = healthMap;
this.parallelResult = parallelResult;
this.countDownLatch = countDownLatch;
}

@Override
public void run() {
try {
if (!HealthCheckerProcessor.this.doHealthCheck(key, value, false, healthMap, true,
false)) {
parallelResult.set(false);
}
} catch (Throwable t) {
parallelResult.set(false);
logger.error(ErrorCode.convert("01-22004"), t);
healthMap.put(key, new Health.Builder().withException(t).status(Status.DOWN)
.build());

Check warning on line 389 in sofa-boot-project/sofa-boot-actuator/src/main/java/com/alipay/sofa/boot/actuator/health/HealthCheckerProcessor.java

View check run for this annotation

Codecov / codecov/patch

sofa-boot-project/sofa-boot-actuator/src/main/java/com/alipay/sofa/boot/actuator/health/HealthCheckerProcessor.java#L385-L389

Added lines #L385 - L389 were not covered by tests
} finally {
countDownLatch.countDown();
}
}
}

private class AsyncHealthCheckCallable implements Callable<Health> {

private final HealthChecker healthChecker;

public AsyncHealthCheckCallable(HealthChecker healthChecker) {
this.healthChecker = healthChecker;
}

@Override
public Health call() throws Exception {
return healthChecker.isHealthy();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -167,19 +168,8 @@
if (isParallelCheck()) {
CountDownLatch countDownLatch = new CountDownLatch(healthIndicators.size());
AtomicBoolean parallelResult = new AtomicBoolean(true);
healthIndicators.forEach((key, value) -> healthCheckExecutor.execute(() -> {
try {
if (!doHealthCheck(key, value, healthMap, false)) {
parallelResult.set(false);
}
} catch (Throwable t) {
parallelResult.set(false);
logger.error(ErrorCode.convert("01-21003"), t);
healthMap.put(key, new Health.Builder().withException(t).status(Status.DOWN).build());
} finally {
countDownLatch.countDown();
}
}));
healthIndicators.forEach((key, value) -> healthCheckExecutor.execute(
new AsyncHealthIndicatorRunnable(key, value, healthMap, parallelResult, countDownLatch)));
Comment on lines +171 to +172
Copy link

Choose a reason for hiding this comment

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

The implementation of parallel health checks using AsyncHealthIndicatorRunnable is effective. Ensure that the timeout handling is robust and consider logging more details about the timeout event.

- healthMap.put("parallelCheck", new Health.Builder().withDetail("timeout", getParallelCheckTimeout()).status(Status.UNKNOWN).build());
+ logger.warn("Parallel health check timed out after {}ms.", getParallelCheckTimeout());
+ healthMap.put("parallelCheck", new Health.Builder().withDetail("timeout", getParallelCheckTimeout()).status(Status.UNKNOWN).build());

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
healthIndicators.forEach((key, value) -> healthCheckExecutor.execute(
new AsyncHealthIndicatorRunnable(key, value, healthMap, parallelResult, countDownLatch)));
healthIndicators.forEach((key, value) -> healthCheckExecutor.execute(
new AsyncHealthIndicatorRunnable(key, value, healthMap, parallelResult, countDownLatch)));
logger.warn("Parallel health check timed out after {}ms.", getParallelCheckTimeout());
healthMap.put("parallelCheck", new Health.Builder().withDetail("timeout", getParallelCheckTimeout()).status(Status.UNKNOWN).build());

boolean finished = false;
try {
finished = countDownLatch.await(getParallelCheckTimeout(), TimeUnit.MILLISECONDS);
Expand Down Expand Up @@ -226,7 +216,7 @@
try {
if (wait) {
Future<Health> future = healthCheckExecutor
.submit(healthIndicator::health);
.submit(new AsyncHealthIndicatorCallable(healthIndicator));
health = future.get(timeout, TimeUnit.MILLISECONDS);
} else {
health = healthIndicator.health();
Expand Down Expand Up @@ -315,4 +305,55 @@
public List<BaseStat> getHealthIndicatorStartupStatList() {
return healthIndicatorStartupStatList;
}

private class AsyncHealthIndicatorRunnable implements Runnable {
private final String key;
private final HealthIndicator value;
private final Map<String, Health> healthMap;

private final AtomicBoolean parallelResult;

private final CountDownLatch countDownLatch;

public AsyncHealthIndicatorRunnable(String key, HealthIndicator value,
Map<String, Health> healthMap,
AtomicBoolean parallelResult,
CountDownLatch countDownLatch) {
this.key = key;
this.value = value;
this.healthMap = healthMap;
this.parallelResult = parallelResult;
this.countDownLatch = countDownLatch;
}

@Override
public void run() {
try {
if (!HealthIndicatorProcessor.this.doHealthCheck(key, value, healthMap, false)) {
parallelResult.set(false);
}
} catch (Throwable t) {
parallelResult.set(false);
logger.error(ErrorCode.convert("01-21003"), t);
healthMap.put(key, new Health.Builder().withException(t).status(Status.DOWN)
.build());

Check warning on line 339 in sofa-boot-project/sofa-boot-actuator/src/main/java/com/alipay/sofa/boot/actuator/health/HealthIndicatorProcessor.java

View check run for this annotation

Codecov / codecov/patch

sofa-boot-project/sofa-boot-actuator/src/main/java/com/alipay/sofa/boot/actuator/health/HealthIndicatorProcessor.java#L335-L339

Added lines #L335 - L339 were not covered by tests
} finally {
countDownLatch.countDown();
}
}
}

private class AsyncHealthIndicatorCallable implements Callable<Health> {

private final HealthIndicator healthIndicator;

public AsyncHealthIndicatorCallable(HealthIndicator healthIndicator) {
this.healthIndicator = healthIndicator;
}

@Override
public Health call() throws Exception {
return healthIndicator.health();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -184,38 +184,12 @@
/**
* Refresh all {@link ApplicationContext} recursively
*/
private void refreshRecursively(DeploymentDescriptor deployment,
CountDownLatch latch, List<Future<?>> futures) {
private void refreshRecursively(DeploymentDescriptor deployment, CountDownLatch latch,
List<Future<?>> futures) {
// if interrupted, moduleRefreshExecutorService will be null;
if (moduleRefreshExecutorService != null) {
futures.add(moduleRefreshExecutorService.submit(() -> {
String oldName = Thread.currentThread().getName();
try {
Thread.currentThread().setName(
"sofa-module-refresh-" + deployment.getModuleName());
if (deployment.isSpringPowered() && !application.getFailed().contains(deployment)) {
refreshAndCollectCost(deployment);
}
DependencyTree.Entry<String, DeploymentDescriptor> entry = application
.getDeployRegistry().getEntry(deployment.getModuleName());
if (entry != null && entry.getDependsOnMe() != null) {
for (DependencyTree.Entry<String, DeploymentDescriptor> child : entry
.getDependsOnMe()) {
child.getDependencies().remove(entry);
if (child.getDependencies().size() == 0) {
refreshRecursively(child.get(), latch, futures);
}
}
}
} catch (Throwable t) {
LOGGER.error(ErrorCode.convert("01-11002", deployment.getName()), t);
throw new RuntimeException(ErrorCode.convert("01-11002", deployment.getName()),
t);
} finally {
latch.countDown();
Thread.currentThread().setName(oldName);
}
}));
futures.add(moduleRefreshExecutorService.submit(new AsyncSpringContextRunnable(
deployment, latch, futures)));
}
}

Expand Down Expand Up @@ -291,4 +265,46 @@
public int getOrder() {
return 20000;
}

private class AsyncSpringContextRunnable implements Runnable {
private final DeploymentDescriptor deployment;
private final CountDownLatch latch;
private final List<Future<?>> futures;

public AsyncSpringContextRunnable(DeploymentDescriptor deployment, CountDownLatch latch,
List<Future<?>> futures) {
this.deployment = deployment;
this.latch = latch;
this.futures = futures;
}

@Override
public void run() {
String oldName = Thread.currentThread().getName();
try {
Thread.currentThread().setName("sofa-module-refresh-" + deployment.getModuleName());
if (deployment.isSpringPowered() && !application.getFailed().contains(deployment)) {
SpringContextInstallStage.this.refreshAndCollectCost(deployment);
}
DependencyTree.Entry<String, DeploymentDescriptor> entry = application
.getDeployRegistry().getEntry(deployment.getModuleName());
if (entry != null && entry.getDependsOnMe() != null) {
for (DependencyTree.Entry<String, DeploymentDescriptor> child : entry
.getDependsOnMe()) {
child.getDependencies().remove(entry);
if (child.getDependencies().size() == 0) {
SpringContextInstallStage.this.refreshRecursively(child.get(), latch,
futures);
}
}
}
} catch (Throwable t) {
LOGGER.error(ErrorCode.convert("01-11002", deployment.getName()), t);
throw new RuntimeException(ErrorCode.convert("01-11002", deployment.getName()), t);

Check warning on line 303 in sofa-boot-project/sofa-boot-core/isle-sofa-boot/src/main/java/com/alipay/sofa/boot/isle/stage/SpringContextInstallStage.java

View check run for this annotation

Codecov / codecov/patch

sofa-boot-project/sofa-boot-core/isle-sofa-boot/src/main/java/com/alipay/sofa/boot/isle/stage/SpringContextInstallStage.java#L301-L303

Added lines #L301 - L303 were not covered by tests
} finally {
latch.countDown();
Thread.currentThread().setName(oldName);
}
}
Comment on lines +269 to +308
Copy link

Choose a reason for hiding this comment

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

The AsyncSpringContextRunnable class is well-implemented. Consider adding error handling within the run method to manage exceptions more gracefully.

- throw new RuntimeException(ErrorCode.convert("01-11002", deployment.getName()), t);
+ LOGGER.error("Error during the refresh of ApplicationContext for module: " + deployment.getName(), t);
+ throw new DeploymentException("Failed to refresh ApplicationContext for module: " + deployment.getName(), t);

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
private class AsyncSpringContextRunnable implements Runnable {
private final DeploymentDescriptor deployment;
private final CountDownLatch latch;
private final List<Future<?>> futures;
public AsyncSpringContextRunnable(DeploymentDescriptor deployment, CountDownLatch latch,
List<Future<?>> futures) {
this.deployment = deployment;
this.latch = latch;
this.futures = futures;
}
@Override
public void run() {
String oldName = Thread.currentThread().getName();
try {
Thread.currentThread().setName("sofa-module-refresh-" + deployment.getModuleName());
if (deployment.isSpringPowered() && !application.getFailed().contains(deployment)) {
SpringContextInstallStage.this.refreshAndCollectCost(deployment);
}
DependencyTree.Entry<String, DeploymentDescriptor> entry = application
.getDeployRegistry().getEntry(deployment.getModuleName());
if (entry != null && entry.getDependsOnMe() != null) {
for (DependencyTree.Entry<String, DeploymentDescriptor> child : entry
.getDependsOnMe()) {
child.getDependencies().remove(entry);
if (child.getDependencies().size() == 0) {
SpringContextInstallStage.this.refreshRecursively(child.get(), latch,
futures);
}
}
}
} catch (Throwable t) {
LOGGER.error(ErrorCode.convert("01-11002", deployment.getName()), t);
throw new RuntimeException(ErrorCode.convert("01-11002", deployment.getName()), t);
} finally {
latch.countDown();
Thread.currentThread().setName(oldName);
}
}
@Override
public void run() {
String oldName = Thread.currentThread().getName();
try {
Thread.currentThread().setName("sofa-module-refresh-" + deployment.getModuleName());
if (deployment.isSpringPowered() && !application.getFailed().contains(deployment)) {
SpringContextInstallStage.this.refreshAndCollectCost(deployment);
}
DependencyTree.Entry<String, DeploymentDescriptor> entry = application
.getDeployRegistry().getEntry(deployment.getModuleName());
if (entry != null && entry.getDependsOnMe() != null) {
for (DependencyTree.Entry<String, DeploymentDescriptor> child : entry
.getDependsOnMe()) {
child.getDependencies().remove(entry);
if (child.getDependencies().size() == 0) {
SpringContextInstallStage.this.refreshRecursively(child.get(), latch,
futures);
}
}
}
} catch (Throwable t) {
LOGGER.error(ErrorCode.convert("01-11002", deployment.getName()), t);
LOGGER.error("Error during the refresh of ApplicationContext for module: " + deployment.getName(), t);
throw new DeploymentException("Failed to refresh ApplicationContext for module: " + deployment.getName(), t);
} finally {
latch.countDown();
Thread.currentThread().setName(oldName);
}
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,28 +75,15 @@
if (!isAsyncCalled && methodName.equals(asyncMethodName)) {
isAsyncCalled = true;
isAsyncCalling = true;
asyncInitMethodManager.submitTask(() -> {
try {
long startTime = System.currentTimeMillis();
invocation.getMethod().invoke(targetObject, invocation.getArguments());
LOGGER.info("{}({}) {} method execute {}dms.", targetObject
.getClass().getName(), beanName, methodName, (System
.currentTimeMillis() - startTime));
} catch (Throwable e) {
throw new RuntimeException(e);
} finally {
asyncMethodFinish();
}
});
asyncInitMethodManager.submitTask(new AsyncBeanInitRunnable(invocation));
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Ensure proper handling of exceptions in the asynchronous task submission.

The search results indicate that there are various instances of exception handling across the codebase, but none specifically around the submitTask method in the AsyncInitializeBeanMethodInvoker class. This suggests that exceptions during task submission might not be handled properly in this specific context.

  • File: sofa-boot-project/sofa-boot-core/runtime-sofa-boot/src/main/java/com/alipay/sofa/runtime/async/AsyncInitializeBeanMethodInvoker.java
  • Line: 78

Please review and ensure that exceptions during the asynchronous task submission are properly handled to avoid potential runtime issues.

Analysis chain

Ensure proper handling of exceptions in the asynchronous task submission.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that exceptions during task submission are handled properly.
# Test: Search for exception handling patterns around task submissions.
rg --type java 'submit.*catch' --context 5

Length of output: 44



Script:

#!/bin/bash
# Description: Verify that exceptions during task submission are handled properly in the AsyncInitializeBeanMethodInvoker class and related classes.
# Test: Search for exception handling patterns in the AsyncInitializeBeanMethodInvoker class and related classes.
rg --type java 'catch' --context 5

Length of output: 261503

return null;
}

if (isAsyncCalling) {
long startTime = System.currentTimeMillis();
initCountDownLatch.await();
LOGGER.info("{}({}) {} method wait {}ms.",
targetObject.getClass().getName(), beanName, methodName,
(System.currentTimeMillis() - startTime));
LOGGER.info("{}({}) {} method wait {}ms.", targetObject.getClass().getName(), beanName,
methodName, (System.currentTimeMillis() - startTime));

Check warning on line 86 in sofa-boot-project/sofa-boot-core/runtime-sofa-boot/src/main/java/com/alipay/sofa/runtime/async/AsyncInitializeBeanMethodInvoker.java

View check run for this annotation

Codecov / codecov/patch

sofa-boot-project/sofa-boot-core/runtime-sofa-boot/src/main/java/com/alipay/sofa/runtime/async/AsyncInitializeBeanMethodInvoker.java#L85-L86

Added lines #L85 - L86 were not covered by tests
Comment on lines +85 to +86
Copy link

Choose a reason for hiding this comment

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

Consider adding more detailed logging for the wait duration to aid in debugging and performance monitoring.

- LOGGER.info("{}({}) {} method wait {}ms.", targetObject.getClass().getName(), beanName, methodName, (System.currentTimeMillis() - startTime));
+ LOGGER.debug("Waiting for async method completion. Class: {}, Bean: {}, Method: {}, Wait Time: {}ms.", targetObject.getClass().getName(), beanName, methodName, (System.currentTimeMillis() - startTime));

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
LOGGER.info("{}({}) {} method wait {}ms.", targetObject.getClass().getName(), beanName,
methodName, (System.currentTimeMillis() - startTime));
LOGGER.debug("Waiting for async method completion. Class: {}, Bean: {}, Method: {}, Wait Time: {}ms.", targetObject.getClass().getName(), beanName, methodName, (System.currentTimeMillis() - startTime));

}
return invocation.getMethod().invoke(targetObject, invocation.getArguments());
}
Expand All @@ -105,4 +92,28 @@
this.initCountDownLatch.countDown();
this.isAsyncCalling = false;
}

private class AsyncBeanInitRunnable implements Runnable {

private final MethodInvocation invocation;

public AsyncBeanInitRunnable(MethodInvocation invocation) {
this.invocation = invocation;
}

@Override
public void run() {
try {
long startTime = System.currentTimeMillis();
invocation.getMethod().invoke(targetObject, invocation.getArguments());
LOGGER.info("{}({}) {} method execute {}dms.", targetObject.getClass().getName(),
beanName, invocation.getMethod().getName(),
(System.currentTimeMillis() - startTime));
} catch (Throwable e) {
throw new RuntimeException(e);

Check warning on line 113 in sofa-boot-project/sofa-boot-core/runtime-sofa-boot/src/main/java/com/alipay/sofa/runtime/async/AsyncInitializeBeanMethodInvoker.java

View check run for this annotation

Codecov / codecov/patch

sofa-boot-project/sofa-boot-core/runtime-sofa-boot/src/main/java/com/alipay/sofa/runtime/async/AsyncInitializeBeanMethodInvoker.java#L112-L113

Added lines #L112 - L113 were not covered by tests
} finally {
AsyncInitializeBeanMethodInvoker.this.asyncMethodFinish();
}
}
}
Comment on lines +96 to +118
Copy link

Choose a reason for hiding this comment

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

The implementation of AsyncBeanInitRunnable is robust. However, consider enhancing error logging by including more context about the failure.

- throw new RuntimeException(e);
+ throw new RuntimeException("Error executing async initialization for method: " + invocation.getMethod().getName() + " in bean: " + beanName, e);

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
private class AsyncBeanInitRunnable implements Runnable {
private final MethodInvocation invocation;
public AsyncBeanInitRunnable(MethodInvocation invocation) {
this.invocation = invocation;
}
@Override
public void run() {
try {
long startTime = System.currentTimeMillis();
invocation.getMethod().invoke(targetObject, invocation.getArguments());
LOGGER.info("{}({}) {} method execute {}dms.", targetObject.getClass().getName(),
beanName, invocation.getMethod().getName(),
(System.currentTimeMillis() - startTime));
} catch (Throwable e) {
throw new RuntimeException(e);
} finally {
AsyncInitializeBeanMethodInvoker.this.asyncMethodFinish();
}
}
}
private class AsyncBeanInitRunnable implements Runnable {
private final MethodInvocation invocation;
public AsyncBeanInitRunnable(MethodInvocation invocation) {
this.invocation = invocation;
}
@Override
public void run() {
try {
long startTime = System.currentTimeMillis();
invocation.getMethod().invoke(targetObject, invocation.getArguments());
LOGGER.info("{}({}) {} method execute {}dms.", targetObject.getClass().getName(),
beanName, invocation.getMethod().getName(),
(System.currentTimeMillis() - startTime));
} catch (Throwable e) {
throw new RuntimeException("Error executing async initialization for method: " + invocation.getMethod().getName() + " in bean: " + beanName, e);
} finally {
AsyncInitializeBeanMethodInvoker.this.asyncMethodFinish();
}
}
}

}
Loading