You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2020/01/20 06:59:49 UTC

[skywalking] branch master updated: Fix potential concurrent problems and tune performance (#4274)

This is an automated email from the ASF dual-hosted git repository.

wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git


The following commit(s) were added to refs/heads/master by this push:
     new 75aefd3  Fix potential concurrent problems and tune performance (#4274)
75aefd3 is described below

commit 75aefd34de1a7ea75daa52020b5f90bc60009a99
Author: kezhenxu94 <ke...@apache.org>
AuthorDate: Mon Jan 20 14:59:41 2020 +0800

    Fix potential concurrent problems and tune performance (#4274)
---
 .../oap/server/core/alarm/provider/AlarmRule.java  |  4 +-
 .../server/core/alarm/provider/RunningRule.java    | 39 +++------
 .../oap/server/core/query/DurationUtils.java       | 97 +++++++++++-----------
 .../oap/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 9b40cce..93f59f0 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<String> includeNames;
+    private ArrayList<String> 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 5fe7471..a39b269 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<MetaInAlarm, Window> windows;
+    private final Map<MetaInAlarm, Window> windows;
     private volatile MetricsValueType valueType;
-    private int targetScopeId;
-    private List<String> includeNames;
-    private List<String> excludeNames;
-    private AlarmMessageFormatter formatter;
+    private final List<String> includeNames;
+    private final List<String> 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<AlarmMessage> check() {
         List<AlarmMessage> 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 00fa126..dd5fc44 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<DurationPoint> getDurationPoints(Downsampling downsampling, long startTimeBucket,
-        long endTimeBucket) throws ParseException {
+        long endTimeBucket) {
         DateTime dateTime = parseToDateTime(downsampling, startTimeBucket);
 
         List<DurationPoint> 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 a53930f..8636edc 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<DurationPoint> durationPoints = DurationUtils.INSTANCE.getDurationPoints(downsampling, startTB, endTB);
         List<String> ids = new ArrayList<>();
         if (StringUtil.isEmpty(id)) {
@@ -99,7 +99,7 @@ public class MetricQueryService implements Service {
     public List<IntValues> 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<DurationPoint> durationPoints = DurationUtils.INSTANCE.getDurationPoints(downsampling, startTB, endTB);
         List<String> 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<IntValues> response = new ArrayList<IntValues>(numOfLinear);
-        for (IntValues value : multipleLinearIntValues) {
-            response.add(value);
-        }
+        List<IntValues> 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<DurationPoint> durationPoints = DurationUtils.INSTANCE.getDurationPoints(downsampling, startTB, endTB);
         List<String> ids = new ArrayList<>();
         durationPoints.forEach(durationPoint -> {