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

fix(core): enhancement AnomalyDetector and LoadRetriever #1035

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

Wzy19930507
Copy link
Contributor

@Wzy19930507 Wzy19930507 commented Mar 27, 2024

close #989

  • Enhancement AnomalyDetector log throwable and exit loop.
  • Enhancement LoadRetriever log throwable and exit loop.

@CLAassistant
Copy link

CLAassistant commented Mar 27, 2024

CLA assistant check
All committers have signed the CLA.

* Enhancement AnomalyDetector always execute
* Enhancement LoadRetriever always execute
@Wzy19930507 Wzy19930507 changed the title close #989 fix(core): enhancement AnomalyDetector and LoadRetriever Mar 27, 2024
@@ -428,8 +428,8 @@ public void retrieve() {
createTopic();
this.mainExecutorService.schedule(this::retrieve, 1, TimeUnit.SECONDS);
return;
} catch (Exception e) {
logger.error("Consumer poll error", e);
} catch (Throwable throwable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We rerun the task on Exception is because Exception is expected to be recoverable, however for non-recoverable cases (like most of the Error), schedule another task is unneccessary becasue the current JVM status is unpredictable, so maybe just add catch Throwable after Exception, and logging it properly

} catch (Exception e) {
logger.error("Consumer poll error", e);
} catch (Throwable throwable) {
logger.error("Consumer poll throwable", throwable);
Copy link
Contributor

Choose a reason for hiding this comment

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

there is no need to change the logging text here, as "throwable" is an adj.

@SCNieh SCNieh merged commit 3c8fe2f into AutoMQ:main Mar 28, 2024
8 checks passed
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

Successfully merging this pull request may close these issues.

[Discuss] Error interrupt LoadRetriever loop
3 participants