[Improvement]: Implement a stateless overview manager. #3429
Conversation
github-actions
bot
added
module:ams-server
# Conflicts: # dist/src/main/amoro-bin/conf/config.yaml
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3429 +/- ##
============================================
+ Coverage 21.80% 27.72% +5.91%
- Complexity 2353 3614 +1261
============================================
Files 431 602 +171
Lines 39948 49141 +9193
Branches 5655 6333 +678
============================================
+ Hits 8712 13623 +4911
- Misses 30504 34569 +4065
- Partials 732 949 +217
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
@baiyangtx Thanks for the work.
I left some comments, PTAL.
| @@ -38,25 +38,25 @@ | |||
| /** The controller that handles overview page requests. */ | |||
| public class OverviewController { | |||
|
|
|||
| private OverviewCache overviewCache; | |||
There was a problem hiding this comment.
OverviewCache seems no longer needed, we may remove it.
| @VisibleForTesting | ||
| public OverviewManager(int maxRecordCount, Duration refreshInterval) { | ||
| this.maxRecordCount = maxRecordCount; | ||
| ScheduledExecutorService overviewUpdaterScheduler = |
There was a problem hiding this comment.
overviewRefreshScheduler may be a proper name, and the thread name pattern can be overview-refresh-scheduler-%d.
| @VisibleForTesting | ||
| public void refresh() { | ||
| long start = System.currentTimeMillis(); | ||
| LOG.info("Updating overview cache"); |
There was a problem hiding this comment.
| LOG.info("Updating overview cache"); | |
| LOG.info("Refreshing overview cache"); |
| refreshResourceUsage(start); | ||
|
|
||
| } catch (Exception e) { | ||
| LOG.error("OverviewRefresher error", e); |
There was a problem hiding this comment.
| LOG.error("OverviewRefresher error", e); | |
| LOG.error("Refreshed overview cache failed", e); |
| LOG.error("OverviewRefresher error", e); | ||
| } finally { | ||
| long end = System.currentTimeMillis(); | ||
| LOG.info("Refresher overview cache took {} ms.", end - start); |
There was a problem hiding this comment.
| LOG.info("Refresher overview cache took {} ms.", end - start); | |
| LOG.info("Refreshed overview cache in {} ms", end - start); |
| }); | ||
| String status = statusToMetricString(meta.getTableStatus()); | ||
| if (StringUtils.isNotEmpty(status)) { | ||
| optimizingStatusMap.computeIfAbsent(status, ignore -> 0L); |
There was a problem hiding this comment.
| optimizingStatusMap.computeIfAbsent(status, ignore -> 0L); | |
| optimizingStatusMap.putIfAbsent(status, 0L); |
Why are the changes needed?
Make overview stateless to deploy it in multi-instance mode.
Brief change log
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation