Bug fix of search logic of metric files (#331)

* Fix #330 : fix listMetricFiles() returns wrong file name list. Signed-off-by: Carpenter Lee <[email protected]> * Add timestamp check when read metrics. Signed-off-by: Carpenter Lee <[email protected]> * Refine test case Signed-off-by: Carpenter Lee <[email protected]>
alibaba · Dec 25, 2018 · 97d5b9a · 97d5b9a
1 parent 4fccbf5
commit 97d5b9a
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 18 deletions.
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricSearcher.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricSearcher.java
@@ -134,7 +134,7 @@ public synchronized List<MetricNode> findByTimeAndResource(long beginTimeMs, lon
                     MetricWriter.formIndexFileName(fileName), offsetInIndex);
             offsetInIndex = 0;
             if (offset != -1) {
-                return metricsReader.readMetricsByEndTime(fileNames, i, offset, endTimeMs, identity);
+                return metricsReader.readMetricsByEndTime(fileNames, i, offset, beginTimeMs, endTimeMs, identity);
             }
         }
         return null;

diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricWriter.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricWriter.java
@@ -194,24 +194,26 @@ private String nextFileNameOfDay(long time) {
         File baseFile = new File(baseDir);
         DateFormat fileNameDf = new SimpleDateFormat("yyyy-MM-dd");
         String dateStr = fileNameDf.format(new Date(time));
+        String fileNameModel = baseFileName + "." + dateStr;
         for (File file : baseFile.listFiles()) {
-            if (file.getName().contains(baseFileName + "." + dateStr)
-                && !file.getName().endsWith(METRIC_FILE_INDEX_SUFFIX)
-                && !file.getName().endsWith(".lck")) {
+            String fileName = file.getName();
+            if (fileName.contains(fileNameModel)
+                && !fileName.endsWith(METRIC_FILE_INDEX_SUFFIX)
+                && !fileName.endsWith(".lck")) {
                 list.add(file.getAbsolutePath());
             }
         }
         Collections.sort(list, METRIC_FILE_NAME_CMP);
         if (list.isEmpty()) {
-            return baseDir + baseFileName + "." + dateStr;
+            return baseDir + fileNameModel;
         }
         String last = list.get(list.size() - 1);
         int n = 0;
         String[] strs = last.split("\\.");
         if (strs.length > 0 && strs[strs.length - 1].matches("[0-9]{1,10}")) {
             n = Integer.parseInt(strs[strs.length - 1]);
         }
-        return baseDir + baseFileName + "." + dateStr + "." + (n + 1);
+        return baseDir + fileNameModel + "." + (n + 1);
     }
 
     /**
@@ -244,7 +246,7 @@ public int compare(String o1, String o2) {
             String name2 = new File(o2).getName();
             String dateStr1 = name1.split("\\.")[2];
             String dateStr2 = name2.split("\\.")[2];
-            // in case of file name contains pid, skip it
+            // in case of file name contains pid, skip it, like Sentinel-Admin-metrics.log.pid22568.2018-12-24
             if (dateStr1.startsWith(pid)) {
                 dateStr1 = name1.split("\\.")[3];
                 dateStr2 = name2.split("\\.")[3];
@@ -266,7 +268,10 @@ public int compare(String o1, String o2) {
     }
 
     /**
-     * Get all metric files' name in {@code baseDir}. The file name must contain {@code baseFileName}
+     * Get all metric files' name in {@code baseDir}. The file name must like
+     * <pre>
+     * baseFileName + ".yyyy-MM-dd.number"
+     * </pre>
      * and not endsWith {@link #METRIC_FILE_INDEX_SUFFIX} or ".lck".
      *
      * @param baseDir      the directory to search.
@@ -282,17 +287,38 @@ static List<String> listMetricFiles(String baseDir, String baseFileName) throws
             return list;
         }
         for (File file : files) {
+            String fileName = file.getName();
             if (file.isFile()
-                && file.getName().contains(baseFileName)
-                && !file.getName().endsWith(MetricWriter.METRIC_FILE_INDEX_SUFFIX)
-                && !file.getName().endsWith(".lck")) {
+                && fileNameMatches(fileName, baseFileName)
+                && !fileName.endsWith(MetricWriter.METRIC_FILE_INDEX_SUFFIX)
+                && !fileName.endsWith(".lck")) {
                 list.add(file.getAbsolutePath());
             }
         }
         Collections.sort(list, MetricWriter.METRIC_FILE_NAME_CMP);
         return list;
     }
 
+    /**
+     * Test whether fileName matches baseFileName. fileName matches baseFileName when
+     * <pre>
+     * fileName = baseFileName + ".yyyy-MM-dd.number"
+     * </pre>
+     *
+     * @param fileName     file name
+     * @param baseFileName base file name.
+     * @return if fileName matches baseFileName return true, else return false.
+     */
+    public static boolean fileNameMatches(String fileName, String baseFileName) {
+        if (fileName.startsWith(baseFileName)) {
+            String part = fileName.substring(baseFileName.length());
+            // part is like: ".yyyy-MM-dd.number", eg. ".2018-12-24.11"
+            return part.matches("\\.[0-9]{4}-[0-9]{2}-[0-9]{2}(\\.[0-9]*)?");
+        } else {
+            return false;
+        }
+    }
+
     private void removeMoreFiles() throws Exception {
         List<String> list = listMetricFiles(baseDir, baseFileName);
         if (list == null || list.isEmpty()) {

diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricsReader.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricsReader.java
@@ -36,9 +36,10 @@ public MetricsReader(Charset charset) {
     /**
      * @return if should continue read, return true, else false.
      */
-    boolean readMetricsInOneFileByEndTime(List<MetricNode> list, String fileName,
-                                          long offset, long endTimeMs, String identity) throws Exception {
+    boolean readMetricsInOneFileByEndTime(List<MetricNode> list, String fileName, long offset,
+                                          long beginTimeMs, long endTimeMs, String identity) throws Exception {
         FileInputStream in = null;
+        long beginSecond = beginTimeMs / 1000;
         long endSecond = endTimeMs / 1000;
         try {
             in = new FileInputStream(fileName);
@@ -48,6 +49,10 @@ boolean readMetricsInOneFileByEndTime(List<MetricNode> list, String fileName,
             while ((line = reader.readLine()) != null) {
                 MetricNode node = MetricNode.fromFatString(line);
                 long currentSecond = node.getTimestamp() / 1000;
+                // currentSecond should >= beginSecond, otherwise a wrong metric file must occur
+                if (currentSecond < beginSecond) {
+                    return false;
+                }
                 if (currentSecond <= endSecond) {
                     // read all
                     if (identity == null) {
@@ -109,12 +114,12 @@ void readMetricsInOneFile(List<MetricNode> list, String fileName,
      * When identity is null, all metric between the time intervalMs will be read, otherwise, only the specific
      * identity will be read.
      */
-    List<MetricNode> readMetricsByEndTime(List<String> fileNames, int pos,
-                                          long offset, long endTimeMs, String identity) throws Exception {
+    List<MetricNode> readMetricsByEndTime(List<String> fileNames, int pos, long offset,
+                                          long beginTimeMs, long endTimeMs, String identity) throws Exception {
         List<MetricNode> list = new ArrayList<MetricNode>(1024);
-        if (readMetricsInOneFileByEndTime(list, fileNames.get(pos++), offset, endTimeMs, identity)) {
+        if (readMetricsInOneFileByEndTime(list, fileNames.get(pos++), offset, beginTimeMs, endTimeMs, identity)) {
             while (pos < fileNames.size()
-                    && readMetricsInOneFileByEndTime(list, fileNames.get(pos++), 0, endTimeMs, identity)) {
+                && readMetricsInOneFileByEndTime(list, fileNames.get(pos++), 0, beginTimeMs, endTimeMs, identity)) {
             }
         }
         return list;

diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/metric/MetricWriterTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/node/metric/MetricWriterTest.java
@@ -6,8 +6,13 @@
 
 import org.junit.Test;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
+/**
+ * @author Carpenter Lee
+ */
 public class MetricWriterTest {
     @Test
     public void testFileNameCmp() {
@@ -51,4 +56,28 @@ public void testFileNamePidCmp() {
         assertEquals(Arrays.asList(key), list);
     }
 
+    @Test
+    public void testFileNameMatches(){
+        String baseFileName = "Sentinel-SDK-Demo-metrics.log";
+        String fileName = "Sentinel-SDK-Demo-metrics.log.2018-03-06";
+        assertTrue(MetricWriter.fileNameMatches(fileName, baseFileName));
+
+        String baseFileName2 = "Sentinel-Admin-metrics.log.pid22568";
+        String fileName2 = "Sentinel-Admin-metrics.log.pid22568.2018-12-24";
+        assertTrue(MetricWriter.fileNameMatches(fileName2, baseFileName2));
+
+        String baseFileName3 = "Sentinel-SDK-Demo-metrics.log";
+        String fileName3 = "Sentinel-SDK-Demo-metrics.log.2018-03-06.11";
+        assertTrue(MetricWriter.fileNameMatches(fileName3, baseFileName3));
+
+        String baseFileName4 = "Sentinel-SDK-Demo-metrics.log";
+        String fileName4 = "Sentinel-SDK-Demo-metrics.log.XXX.2018-03-06.11";
+        assertFalse(MetricWriter.fileNameMatches(fileName4, baseFileName4));
+
+        String baseFileName5 = "Sentinel-SDK-Demo-metrics.log";
+        String fileName5 = "Sentinel-SDK-Demo-metrics.log.2018-03-06.11XXX";
+        assertFalse(MetricWriter.fileNameMatches(fileName5, baseFileName5));
+    }
+
+
 }