This is an automated email from the ASF dual-hosted git repository. haonan pushed a commit to branch jira672 in repository https://gitbox.apache.org/repos/asf/incubator-iotdb.git
commit 7f759602131cce962455dc3e20ea9c21ad1c6938 Author: HTHou <[email protected]> AuthorDate: Thu Jun 4 10:29:31 2020 +0800 fix a delete storage group bug --- .../org/apache/iotdb/db/metadata/MManager.java | 31 +++++++++------------- .../apache/iotdb/db/qp/executor/PlanExecutor.java | 11 +++----- .../iotdb/db/metadata/MManagerBasicTest.java | 18 ++++++------- 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/org/apache/iotdb/db/metadata/MManager.java b/server/src/main/java/org/apache/iotdb/db/metadata/MManager.java index e7ae081..c04f865 100644 --- a/server/src/main/java/org/apache/iotdb/db/metadata/MManager.java +++ b/server/src/main/java/org/apache/iotdb/db/metadata/MManager.java @@ -242,12 +242,9 @@ public class MManager { createTimeseries(plan, offset); break; case MetadataOperationType.DELETE_TIMESERIES: - Pair<Set<String>, String> pair = deleteTimeseries(args[1]); - for (String deleteStorageGroup : pair.left) { - StorageEngine.getInstance().deleteAllDataFilesInOneStorageGroup(deleteStorageGroup); - } - if (!pair.right.isEmpty()) { - throw new DeleteFailedException(pair.right); + String failedTimeseries = deleteTimeseries(args[1], true); + if (!failedTimeseries.isEmpty()) { + throw new DeleteFailedException(failedTimeseries); } break; case MetadataOperationType.SET_STORAGE_GROUP: @@ -356,11 +353,10 @@ public class MManager { * Delete all timeseries under the given path, may cross different storage group * * @param prefixPath path to be deleted, could be root or a prefix path or a full path - * @return 1. The Set contains StorageGroups that contain no more timeseries after this deletion - * files of such StorageGroups should be deleted to reclaim disk space. 2. The String is the - * deletion failed Timeseries + * @param isRecovering indicate if the deletion occur in recovering + * @return The String is the deletion failed Timeseries */ - public Pair<Set<String>, String> deleteTimeseries(String prefixPath) throws MetadataException { + public String deleteTimeseries(String prefixPath, boolean isRecovering) throws MetadataException { lock.writeLock().lock(); if (isStorageGroup(prefixPath)) { @@ -377,8 +373,6 @@ public class MManager { mNodeCache.clear(); } try { - Set<String> emptyStorageGroups = new HashSet<>(); - List<String> allTimeseries = mtree.getAllTimeseriesName(prefixPath); // Monitor storage group seriesPath is not allowed to be deleted allTimeseries.removeIf(p -> p.startsWith(MonitorConstants.STAT_STORAGE_GROUP_PREFIX)); @@ -387,14 +381,17 @@ public class MManager { for (String p : allTimeseries) { try { String emptyStorageGroup = deleteOneTimeseriesAndUpdateStatisticsAndLog(p); - if (emptyStorageGroup != null) { - emptyStorageGroups.add(emptyStorageGroup); + if (!isRecovering && emptyStorageGroup != null) { + StorageEngine.getInstance().deleteAllDataFilesInOneStorageGroup(emptyStorageGroup); + } + if (writeToLog) { + logWriter.deleteTimeseries(p); } } catch (DeleteFailedException e) { failedNames.add(e.getName()); } } - return new Pair<>(emptyStorageGroups, String.join(",", failedNames)); + return String.join(",", failedNames); } catch (IOException e) { throw new MetadataException(e.getMessage()); } finally { @@ -456,10 +453,6 @@ public class MManager { .ifPresent(val -> maxSeriesNumberAmongStorageGroup = val); } } - - if (writeToLog) { - logWriter.deleteTimeseries(path); - } return storageGroupName; } finally { lock.writeLock().unlock(); diff --git a/server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java b/server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java index dde4957..97b0d90 100644 --- a/server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java +++ b/server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java @@ -1144,18 +1144,13 @@ public class PlanExecutor implements IPlanExecutor { List<Path> deletePathList = deleteTimeSeriesPlan.getPaths(); try { deleteDataOfTimeSeries(deletePathList); - Set<String> emptyStorageGroups = new HashSet<>(); List<String> failedNames = new LinkedList<>(); for (Path path : deletePathList) { - Pair<Set<String>, String> pair = mManager.deleteTimeseries(path.toString()); - emptyStorageGroups.addAll(pair.left); - if (!pair.right.isEmpty()) { - failedNames.add(pair.right); + String failedTimeseries = mManager.deleteTimeseries(path.toString(), false); + if (!failedTimeseries.isEmpty()) { + failedNames.add(failedTimeseries); } } - for (String deleteStorageGroup : emptyStorageGroups) { - StorageEngine.getInstance().deleteAllDataFilesInOneStorageGroup(deleteStorageGroup); - } if (!failedNames.isEmpty()) { throw new DeleteFailedException(String.join(",", failedNames)); } diff --git a/server/src/test/java/org/apache/iotdb/db/metadata/MManagerBasicTest.java b/server/src/test/java/org/apache/iotdb/db/metadata/MManagerBasicTest.java index cbd14d4..252d33f 100644 --- a/server/src/test/java/org/apache/iotdb/db/metadata/MManagerBasicTest.java +++ b/server/src/test/java/org/apache/iotdb/db/metadata/MManagerBasicTest.java @@ -117,7 +117,7 @@ public class MManagerBasicTest { } try { - manager.deleteTimeseries("root.laptop.d1.s1"); + manager.deleteTimeseries("root.laptop.d1.s1", false); } catch (MetadataException e) { e.printStackTrace(); fail(e.getMessage()); @@ -125,7 +125,7 @@ public class MManagerBasicTest { assertFalse(manager.isPathExist("root.laptop.d1.s1")); try { - manager.deleteTimeseries("root.laptop.d1.s0"); + manager.deleteTimeseries("root.laptop.d1.s0", false); } catch (MetadataException e) { e.printStackTrace(); fail(e.getMessage()); @@ -155,13 +155,13 @@ public class MManagerBasicTest { assertFalse(manager.checkStorageGroupByPath("root.laptop.d2")); try { - manager.deleteTimeseries("root.laptop.d1.s0"); + manager.deleteTimeseries("root.laptop.d1.s0", false); } catch (MetadataException e) { e.printStackTrace(); fail(e.getMessage()); } try { - manager.deleteTimeseries("root.laptop.d1.s1"); + manager.deleteTimeseries("root.laptop.d1.s1", false); } catch (MetadataException e) { e.printStackTrace(); fail(e.getMessage()); @@ -177,9 +177,9 @@ public class MManagerBasicTest { } try { - manager.deleteTimeseries("root.laptop.d1.1_2"); - manager.deleteTimeseries("root.laptop.d1.\"1.2.3\""); - manager.deleteTimeseries("root.1.2.3"); + manager.deleteTimeseries("root.laptop.d1.1_2", false); + manager.deleteTimeseries("root.laptop.d1.\"1.2.3\"", false); + manager.deleteTimeseries("root.1.2.3", false); } catch (MetadataException e) { e.printStackTrace(); fail(e.getMessage()); @@ -295,9 +295,9 @@ public class MManagerBasicTest { CompressionType.GZIP, null); assertEquals(2, manager.getMaximalSeriesNumberAmongStorageGroups()); - manager.deleteTimeseries("root.laptop.d1.s1"); + manager.deleteTimeseries("root.laptop.d1.s1", false); assertEquals(1, manager.getMaximalSeriesNumberAmongStorageGroups()); - manager.deleteTimeseries("root.laptop.d1.s2"); + manager.deleteTimeseries("root.laptop.d1.s2", false); assertEquals(1, manager.getMaximalSeriesNumberAmongStorageGroups()); }
