[Enhancement] set grace period for mv checker #34850
Conversation
![starrocks-cr[bot]](https://avatars.githubusercontent.com/in/404334?s=60&v=4){kind=small}
| return Duration.ofMinutes(expBackoff); | ||
| } | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
The most risky bug in this code is:
There is a potential RuntimeException being thrown without being handled properly within the tryToActivate method.
The exception handling logic in the try block of the tryToActivate method catches a generic Exception e and logs a warning, but it then propagates the exception up the call stack by throwing a new RuntimeException. If the goal is to prevent activation failures from crashing the MVActiveChecker process, this re-throwing should be removed.
You can modify the code like this:
try {
ConnectContext connect = createConnectContext(db.getId(), dbName.get());
connect.executeSql(sql);
if (mv.isActive()) {
activeOk = true;
LOG.info("[MVActiveChecker] activate MV {} successfully", mvFullName);
} else {
LOG.warn("[MVActiveChecker] activate MV {} failed", mvFullName);
}
} catch (Exception e) {
// Rather than throw a RuntimeException which could disrupt higher level processes,
// it might make sense to only log the error here and handle it gracefully.
LOG.warn("[MVActiveChecker] activate MV {} failed", mvFullName, e);
} finally {
ConnectContext.remove();
}By changing the code like this, the method logs the error and allows for a more graceful failure that the system can recover from, without disrupting the operation of the MVActiveChecker or wider system.
|
@Mergifyio rebase |
Signed-off-by: Murphy <mofei@starrocks.com>
Signed-off-by: Murphy <mofei@starrocks.com>
✅ Branch has been successfully rebased |
murphyatwork
force-pushed
the
murphy_cooldown_checker
branch
from
47f6c62 to
c9b3f71
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
[FE Incremental Coverage Report]✅ pass : 31 / 32 (96.88%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
|
@Mergifyio backport branch-3.2 |
|
@Mergifyio backport branch-3.1 |
|
@Mergifyio backport branch-3.0 |
✅ Backports have been created
|
✅ Backports have been created
|
✅ Backports have been created
|
Signed-off-by: Murphy <mofei@starrocks.com> (cherry picked from commit ea7ebdc) # Conflicts: # fe/fe-core/src/test/java/com/starrocks/analysis/AlterMaterializedViewTest.java
Signed-off-by: Murphy <mofei@starrocks.com> (cherry picked from commit ea7ebdc)
Signed-off-by: Murphy <mofei@starrocks.com> (cherry picked from commit ea7ebdc) # Conflicts: # fe/fe-core/src/main/java/com/starrocks/scheduler/MVActiveChecker.java # fe/fe-core/src/test/java/com/starrocks/analysis/AlterMaterializedViewTest.java
Signed-off-by: Murphy <mofei@starrocks.com> (cherry picked from commit ea7ebdc)
Signed-off-by: Murphy <mofei@starrocks.com> (cherry picked from commit ea7ebdc) # Conflicts: # fe/fe-core/src/test/java/com/starrocks/analysis/AlterMaterializedViewTest.java
Why I'm doing:
What I'm doing:
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: