From 75aefd34de1a7ea75daa52020b5f90bc60009a99 Mon Sep 17 00:00:00 2001 From: kezhenxu94 Date: Mon, 20 Jan 2020 14:59:41 +0800 Subject: [PATCH] Fix potential concurrent problems and tune performance (#4274) --- .../server/core/alarm/provider/AlarmRule.java | 4 +- .../core/alarm/provider/RunningRule.java | 39 +++----- .../oap/server/core/query/DurationUtils.java | 97 ++++++++++--------- .../server/core/query/MetricQueryService.java | 16 ++- 4 files changed, 71 insertions(+), 85 deletions(-) diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRule.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRule.java index 9b40cce5c6..93f59f0ba2 100644 --- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRule.java +++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRule.java @@ -40,8 +40,8 @@ public class AlarmRule { private String alarmRuleName; private String metricsName; - private ArrayList includeNames; - private ArrayList excludeNames; + private ArrayList includeNames; + private ArrayList excludeNames; private String threshold; private String op; private int period; diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java index 5fe747155c..a39b2694d0 100644 --- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java +++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRule.java @@ -26,8 +26,6 @@ import org.joda.time.LocalDateTime; import org.joda.time.Minutes; import org.joda.time.format.DateTimeFormat; import org.joda.time.format.DateTimeFormatter; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.LinkedList; @@ -42,22 +40,20 @@ import java.util.concurrent.locks.ReentrantLock; * @author wusheng */ public class RunningRule { - private static final Logger logger = LoggerFactory.getLogger(RunningRule.class); private static DateTimeFormatter TIME_BUCKET_FORMATTER = DateTimeFormat.forPattern("yyyyMMddHHmm"); - private String ruleName; - private int period; - private String metricsName; + private final String ruleName; + private final int period; + private final String metricsName; private final Threshold threshold; private final OP op; private final int countThreshold; private final int silencePeriod; - private Map windows; + private final Map windows; private volatile MetricsValueType valueType; - private int targetScopeId; - private List includeNames; - private List excludeNames; - private AlarmMessageFormatter formatter; + private final List includeNames; + private final List excludeNames; + private final AlarmMessageFormatter formatter; public RunningRule(AlarmRule alarmRule) { metricsName = alarmRule.getMetricsName(); @@ -120,15 +116,10 @@ public class RunningRule { } else { return; } - targetScopeId = meta.getScopeId(); } if (valueType != null) { - Window window = windows.get(meta); - if (window == null) { - window = new Window(period); - windows.put(meta, window); - } + Window window = windows.computeIfAbsent(meta, ignored -> new Window(period)); window.add(metrics); } } @@ -148,9 +139,7 @@ public class RunningRule { public List check() { List alarmMessageList = new ArrayList<>(30); - windows.entrySet().forEach(entry -> { - MetaInAlarm meta = entry.getKey(); - Window window = entry.getValue(); + windows.forEach((meta, window) -> { AlarmMessage alarmMessage = window.checkAlarm(); if (alarmMessage != AlarmMessage.NONE) { alarmMessage.setScopeId(meta.getScopeId()); @@ -196,7 +185,6 @@ public class RunningRule { try { if (endTime == null) { init(); - endTime = current; } else { int minutes = Minutes.minutesBetween(endTime, current).getMinutes(); if (minutes <= 0) { @@ -211,8 +199,8 @@ public class RunningRule { values.addLast(null); } } - endTime = current; } + endTime = current; } finally { lock.unlock(); } @@ -249,7 +237,7 @@ public class RunningRule { public AlarmMessage checkAlarm() { if (isMatch()) { - /** + /* * When * 1. Metrics value threshold triggers alarm by rule * 2. Counter reaches the count threshold; @@ -260,8 +248,7 @@ public class RunningRule { silenceCountdown = silencePeriod; // set empty message, but new message - AlarmMessage message = new AlarmMessage(); - return message; + return new AlarmMessage(); } else { silenceCountdown--; } @@ -329,7 +316,7 @@ public class RunningRule { } private void init() { - values = new LinkedList(); + values = new LinkedList<>(); for (int i = 0; i < period; i++) { values.add(null); } diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java index 00fa126417..dd5fc44f6d 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/DurationUtils.java @@ -18,13 +18,15 @@ package org.apache.skywalking.oap.server.core.query; -import java.text.*; -import java.util.*; - -import org.apache.skywalking.oap.server.core.*; +import java.util.LinkedList; +import java.util.List; +import org.apache.skywalking.oap.server.core.Const; +import org.apache.skywalking.oap.server.core.UnexpectedException; import org.apache.skywalking.oap.server.core.analysis.Downsampling; import org.apache.skywalking.oap.server.core.query.entity.Step; import org.joda.time.DateTime; +import org.joda.time.format.DateTimeFormat; +import org.joda.time.format.DateTimeFormatter; /** * @author peng-yongsheng @@ -32,10 +34,22 @@ import org.joda.time.DateTime; public enum DurationUtils { INSTANCE; + private static final DateTimeFormatter YYYY_MM = DateTimeFormat.forPattern("yyyy-MM"); + private static final DateTimeFormatter YYYY_MM_DD = DateTimeFormat.forPattern("yyyy-MM-dd"); + private static final DateTimeFormatter YYYY_MM_DD_HH = DateTimeFormat.forPattern("yyyy-MM-dd HH"); + private static final DateTimeFormatter YYYY_MM_DD_HHMM = DateTimeFormat.forPattern("yyyy-MM-dd HHmm"); + private static final DateTimeFormatter YYYY_MM_DD_HHMMSS = DateTimeFormat.forPattern("yyyy-MM-dd HHmmss"); + + private static final DateTimeFormatter YYYYMM = DateTimeFormat.forPattern("yyyyMM"); + private static final DateTimeFormatter YYYYMMDD = DateTimeFormat.forPattern("yyyyMMdd"); + private static final DateTimeFormatter YYYYMMDDHH = DateTimeFormat.forPattern("yyyyMMddHH"); + private static final DateTimeFormatter YYYYMMDDHHMM = DateTimeFormat.forPattern("yyyyMMddHHmm"); + private static final DateTimeFormatter YYYYMMDDHHMMSS = DateTimeFormat.forPattern("yyyyMMddHHmmss"); + public long exchangeToTimeBucket(String dateStr) { dateStr = dateStr.replaceAll(Const.LINE, Const.EMPTY_STRING); dateStr = dateStr.replaceAll(Const.SPACE, Const.EMPTY_STRING); - return Long.valueOf(dateStr); + return Long.parseLong(dateStr); } public long startTimeDurationToSecondTimeBucket(Step step, String dateStr) { @@ -82,34 +96,34 @@ public enum DurationUtils { return secondTimeBucket; } - public long startTimeToTimestamp(Step step, String dateStr) throws ParseException { + public long startTimeToTimestamp(Step step, String dateStr) { switch (step) { case MONTH: - return new SimpleDateFormat("yyyy-MM").parse(dateStr).getTime(); + return YYYY_MM.parseMillis(dateStr); case DAY: - return new SimpleDateFormat("yyyy-MM-dd").parse(dateStr).getTime(); + return YYYY_MM_DD.parseMillis(dateStr); case HOUR: - return new SimpleDateFormat("yyyy-MM-dd HH").parse(dateStr).getTime(); + return YYYY_MM_DD_HH.parseMillis(dateStr); case MINUTE: - return new SimpleDateFormat("yyyy-MM-dd HHmm").parse(dateStr).getTime(); + return YYYY_MM_DD_HHMM.parseMillis(dateStr); case SECOND: - return new SimpleDateFormat("yyyy-MM-dd HHmmss").parse(dateStr).getTime(); + return YYYY_MM_DD_HHMMSS.parseMillis(dateStr); } throw new UnexpectedException("Unsupported step " + step.name()); } - public long endTimeToTimestamp(Step step, String dateStr) throws ParseException { + public long endTimeToTimestamp(Step step, String dateStr) { switch (step) { case MONTH: - return new DateTime(new SimpleDateFormat("yyyy-MM").parse(dateStr)).plusMonths(1).getMillis(); + return YYYY_MM.parseDateTime(dateStr).plusMonths(1).getMillis(); case DAY: - return new DateTime(new SimpleDateFormat("yyyy-MM-dd").parse(dateStr)).plusDays(1).getMillis(); + return YYYY_MM_DD.parseDateTime(dateStr).plusDays(1).getMillis(); case HOUR: - return new DateTime(new SimpleDateFormat("yyyy-MM-dd HH").parse(dateStr)).plusHours(1).getMillis(); + return YYYY_MM_DD_HH.parseDateTime(dateStr).plusHours(1).getMillis(); case MINUTE: - return new DateTime(new SimpleDateFormat("yyyy-MM-dd HHmm").parse(dateStr)).plusMinutes(1).getMillis(); + return YYYY_MM_DD_HHMM.parseDateTime(dateStr).plusMinutes(1).getMillis(); case SECOND: - return new DateTime(new SimpleDateFormat("yyyy-MM-dd HHmmss").parse(dateStr)).plusSeconds(1).getMillis(); + return YYYY_MM_DD_HHMMSS.parseDateTime(dateStr).plusSeconds(1).getMillis(); } throw new UnexpectedException("Unsupported step " + step.name()); } @@ -143,7 +157,7 @@ public enum DurationUtils { } public List getDurationPoints(Downsampling downsampling, long startTimeBucket, - long endTimeBucket) throws ParseException { + long endTimeBucket) { DateTime dateTime = parseToDateTime(downsampling, startTimeBucket); List durations = new LinkedList<>(); @@ -154,28 +168,28 @@ public enum DurationUtils { switch (downsampling) { case Month: dateTime = dateTime.plusMonths(1); - String timeBucket = new SimpleDateFormat("yyyyMM").format(dateTime.toDate()); - durations.add(new DurationPoint(Long.valueOf(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); + String timeBucket = YYYYMM.print(dateTime); + durations.add(new DurationPoint(Long.parseLong(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); break; case Day: dateTime = dateTime.plusDays(1); - timeBucket = new SimpleDateFormat("yyyyMMdd").format(dateTime.toDate()); - durations.add(new DurationPoint(Long.valueOf(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); + timeBucket = YYYYMMDD.print(dateTime); + durations.add(new DurationPoint(Long.parseLong(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); break; case Hour: dateTime = dateTime.plusHours(1); - timeBucket = new SimpleDateFormat("yyyyMMddHH").format(dateTime.toDate()); - durations.add(new DurationPoint(Long.valueOf(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); + timeBucket = YYYYMMDDHH.print(dateTime); + durations.add(new DurationPoint(Long.parseLong(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); break; case Minute: dateTime = dateTime.plusMinutes(1); - timeBucket = new SimpleDateFormat("yyyyMMddHHmm").format(dateTime.toDate()); - durations.add(new DurationPoint(Long.valueOf(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); + timeBucket = YYYYMMDDHHMM.print(dateTime); + durations.add(new DurationPoint(Long.parseLong(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); break; case Second: dateTime = dateTime.plusSeconds(1); - timeBucket = new SimpleDateFormat("yyyyMMddHHmmss").format(dateTime.toDate()); - durations.add(new DurationPoint(Long.valueOf(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); + timeBucket = YYYYMMDDHHMMSS.print(dateTime); + durations.add(new DurationPoint(Long.parseLong(timeBucket), secondsBetween(downsampling, dateTime), minutesBetween(downsampling, dateTime))); break; } i++; @@ -188,32 +202,19 @@ public enum DurationUtils { return durations; } - private DateTime parseToDateTime(Downsampling downsampling, long time) throws ParseException { - DateTime dateTime = null; - + private DateTime parseToDateTime(Downsampling downsampling, long time) { switch (downsampling) { case Month: - Date date = new SimpleDateFormat("yyyyMM").parse(String.valueOf(time)); - dateTime = new DateTime(date); - break; + return YYYYMM.parseDateTime(String.valueOf(time)); case Day: - date = new SimpleDateFormat("yyyyMMdd").parse(String.valueOf(time)); - dateTime = new DateTime(date); - break; + return YYYYMMDD.parseDateTime(String.valueOf(time)); case Hour: - date = new SimpleDateFormat("yyyyMMddHH").parse(String.valueOf(time)); - dateTime = new DateTime(date); - break; + return YYYYMMDDHH.parseDateTime(String.valueOf(time)); case Minute: - date = new SimpleDateFormat("yyyyMMddHHmm").parse(String.valueOf(time)); - dateTime = new DateTime(date); - break; + return YYYYMMDDHHMM.parseDateTime(String.valueOf(time)); case Second: - date = new SimpleDateFormat("yyyyMMddHHmmss").parse(String.valueOf(time)); - dateTime = new DateTime(date); - break; + return YYYYMMDDHHMMSS.parseDateTime(String.valueOf(time)); } - - return dateTime; + throw new UnexpectedException("Unexpected downsampling: " + downsampling.name()); } } diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/MetricQueryService.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/MetricQueryService.java index a53930f852..8636edcda3 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/MetricQueryService.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/MetricQueryService.java @@ -19,8 +19,8 @@ package org.apache.skywalking.oap.server.core.query; import java.io.IOException; -import java.text.ParseException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.apache.skywalking.apm.util.StringUtil; import org.apache.skywalking.oap.server.core.Const; @@ -64,7 +64,7 @@ public class MetricQueryService implements Service { final long startTB, final long endTB) throws IOException { if (CollectionUtils.isEmpty(ids)) { - /** + /* * Don't support query values w/o ID. but UI still did this(as bug), * we return an empty list, and a debug level log, * rather than an exception, which always being considered as a serious error from new users. @@ -84,7 +84,7 @@ public class MetricQueryService implements Service { public IntValues getLinearIntValues(final String indName, final String id, final Downsampling downsampling, final long startTB, - final long endTB) throws IOException, ParseException { + final long endTB) throws IOException { List durationPoints = DurationUtils.INSTANCE.getDurationPoints(downsampling, startTB, endTB); List ids = new ArrayList<>(); if (StringUtil.isEmpty(id)) { @@ -99,7 +99,7 @@ public class MetricQueryService implements Service { public List getMultipleLinearIntValues(final String indName, final String id, final int numOfLinear, final Downsampling downsampling, final long startTB, - final long endTB) throws IOException, ParseException { + final long endTB) throws IOException { List durationPoints = DurationUtils.INSTANCE.getDurationPoints(downsampling, startTB, endTB); List ids = new ArrayList<>(); if (StringUtil.isEmpty(id)) { @@ -110,16 +110,14 @@ public class MetricQueryService implements Service { IntValues[] multipleLinearIntValues = getMetricQueryDAO().getMultipleLinearIntValues(indName, downsampling, ids, numOfLinear, ValueColumnIds.INSTANCE.getValueCName(indName)); - ArrayList response = new ArrayList(numOfLinear); - for (IntValues value : multipleLinearIntValues) { - response.add(value); - } + List response = new ArrayList<>(numOfLinear); + Collections.addAll(response, multipleLinearIntValues); return response; } public Thermodynamic getThermodynamic(final String indName, final String id, final Downsampling downsampling, final long startTB, - final long endTB) throws IOException, ParseException { + final long endTB) throws IOException { List durationPoints = DurationUtils.INSTANCE.getDurationPoints(downsampling, startTB, endTB); List ids = new ArrayList<>(); durationPoints.forEach(durationPoint -> { -- GitLab