From 6387da4418c552f68dfadc5f915100033fca1e78 Mon Sep 17 00:00:00 2001 From: sadanand48 Date: Tue, 24 Nov 2020 11:14:00 +0530 Subject: [PATCH 1/7] HDDS-4451.Handle start & stop of Trash Emptier thread when node becomes leader/follower --- .../hadoop/ozone/shell/TestOzoneShellHA.java | 2 - .../apache/hadoop/ozone/om/OzoneManager.java | 6 +-- .../apache/hadoop/ozone/om/OzoneTrash.java | 39 +++++++++++++++++++ .../hadoop/ozone/om/TrashPolicyOzone.java | 10 ++++- 4 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneTrash.java diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java index 830a3d652f99..4a91912e2806 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java @@ -55,7 +55,6 @@ import static org.junit.Assert.fail; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.Timeout; @@ -467,7 +466,6 @@ private OzoneConfiguration getClientConfForOFS( } @Test - @Ignore("HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily") public void testDeleteToTrashOrSkipTrash() throws Exception { final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId; OzoneConfiguration clientConf = getClientConfForOFS(hostPrefix, conf); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index b5191cede2fc..62e088c93345 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -57,8 +57,6 @@ import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.Trash; -import org.apache.hadoop.fs.TrashPolicy; import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.annotation.InterfaceAudience; @@ -1289,9 +1287,7 @@ public FileSystem run() throws IOException { return FileSystem.get(fsconf); } }); - conf.setClass("fs.trash.classname", TrashPolicyOzone.class, - TrashPolicy.class); - this.emptier = new Thread(new Trash(fs, conf). + this.emptier = new Thread(new OzoneTrash(fs, conf, this). getEmptier(), "Trash Emptier"); this.emptier.setDaemon(true); this.emptier.start(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneTrash.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneTrash.java new file mode 100644 index 000000000000..d0eee36d610f --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneTrash.java @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Trash; + +import java.io.IOException; + +/** + * OzoneTrash which takes an OM as parameter . + */ +public class OzoneTrash extends Trash { + + private TrashPolicyOzone trashPolicy; + public OzoneTrash(FileSystem fs, Configuration conf, OzoneManager om) + throws IOException { + super(fs, conf); + this.trashPolicy = new TrashPolicyOzone(fs, conf, om); + } + +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java index c0278bc291d8..13429bc54325 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java @@ -61,6 +61,8 @@ public class TrashPolicyOzone extends TrashPolicyDefault { private long emptierInterval; + private OzoneManager om; + public TrashPolicyOzone(){ } @@ -68,6 +70,11 @@ private TrashPolicyOzone(FileSystem fs, Configuration conf){ super.initialize(conf, fs); } + TrashPolicyOzone(FileSystem fs, Configuration conf, OzoneManager om){ + super.initialize(conf, fs); + this.om = om; + } + @Override public Runnable getEmptier() throws IOException { return new TrashPolicyOzone.Emptier(getConf(), emptierInterval); @@ -98,7 +105,8 @@ protected class Emptier implements Runnable { @Override public void run() { - if (emptierInterval == 0) { + // if this is not the leader node,don't run the emptier + if (emptierInterval == 0 || !om.isLeader()) { return; // trash disabled } long now, end; From c7baaa7f29bbeb995be69ea03155f7013d771d2d Mon Sep 17 00:00:00 2001 From: sadanand48 Date: Tue, 24 Nov 2020 22:26:32 +0530 Subject: [PATCH 2/7] addressed some comments --- .../apache/hadoop/ozone/om/OzoneTrash.java | 7 +++- .../hadoop/ozone/om/TrashPolicyOzone.java | 34 ++++++++++++++++--- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneTrash.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneTrash.java index d0eee36d610f..310c3012ec79 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneTrash.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneTrash.java @@ -21,6 +21,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Trash; +import org.apache.hadoop.fs.TrashPolicy; import java.io.IOException; @@ -29,11 +30,15 @@ */ public class OzoneTrash extends Trash { - private TrashPolicyOzone trashPolicy; + private TrashPolicy trashPolicy; public OzoneTrash(FileSystem fs, Configuration conf, OzoneManager om) throws IOException { super(fs, conf); this.trashPolicy = new TrashPolicyOzone(fs, conf, om); } + @Override + public Runnable getEmptier() throws IOException { + return this.trashPolicy.getEmptier(); + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java index 13429bc54325..d532619a1e15 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java @@ -37,6 +37,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_DEFAULT; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_CHECKPOINT_INTERVAL_KEY; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_CHECKPOINT_INTERVAL_DEFAULT; + /** * TrashPolicy for Ozone Specific Trash Operations.Through this implementation * of TrashPolicy ozone-specific trash optimizations are/will be made such as @@ -61,23 +66,39 @@ public class TrashPolicyOzone extends TrashPolicyDefault { private long emptierInterval; + private Configuration configuration; + private OzoneManager om; public TrashPolicyOzone(){ } - private TrashPolicyOzone(FileSystem fs, Configuration conf){ - super.initialize(conf, fs); + @Override + public void initialize(Configuration conf, FileSystem fs) { + this.fs = fs; + this.configuration = conf; + this.deletionInterval = (long)(conf.getFloat( + FS_TRASH_INTERVAL_KEY, FS_TRASH_INTERVAL_DEFAULT) + * MSECS_PER_MINUTE); + this.emptierInterval = (long)(conf.getFloat( + FS_TRASH_CHECKPOINT_INTERVAL_KEY, FS_TRASH_CHECKPOINT_INTERVAL_DEFAULT) + * MSECS_PER_MINUTE); + if (deletionInterval < 0) { + LOG.warn("Invalid value {} for deletion interval," + + " deletion interaval can not be negative." + + "Changing to default value 0", deletionInterval); + this.deletionInterval = 0; + } } TrashPolicyOzone(FileSystem fs, Configuration conf, OzoneManager om){ - super.initialize(conf, fs); + initialize(conf, fs); this.om = om; } @Override public Runnable getEmptier() throws IOException { - return new TrashPolicyOzone.Emptier(getConf(), emptierInterval); + return new TrashPolicyOzone.Emptier(configuration, emptierInterval); } protected class Emptier implements Runnable { @@ -115,6 +136,9 @@ public void run() { end = ceiling(now, emptierInterval); try { // sleep for interval Thread.sleep(end - now); + if (!om.isLeader()){ + return; + } } catch (InterruptedException e) { break; // exit on interrupt } @@ -130,7 +154,7 @@ public void run() { continue; } try { - TrashPolicyOzone trash = new TrashPolicyOzone(fs, conf); + TrashPolicyOzone trash = new TrashPolicyOzone(fs, conf, om); trash.deleteCheckpoint(trashRoot.getPath(), false); trash.createCheckpoint(trashRoot.getPath(), new Date(now)); } catch (IOException e) { From 3c85a4a1c32a3dad761e8847b5bb2ecd4372fea2 Mon Sep 17 00:00:00 2001 From: sadanand48 Date: Fri, 11 Dec 2020 10:51:15 +0530 Subject: [PATCH 3/7] addressed comments --- .../apache/hadoop/ozone/om/OzoneManager.java | 19 +++++++++++-------- .../hadoop/ozone/om/TrashPolicyOzone.java | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 62e088c93345..037cea46e8e9 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1185,8 +1185,6 @@ public void start() throws IOException { } omRpcServer.start(); isOmRpcServerRunning = true; - // TODO: Start this thread only on the leader node. - // Should be fixed after HDDS-4451. startTrashEmptier(configuration); registerMXBean(); @@ -1245,8 +1243,6 @@ public void restart() throws IOException { omRpcServer.start(); isOmRpcServerRunning = true; - // TODO: Start this thread only on the leader node. - // Should be fixed after HDDS-4451. startTrashEmptier(configuration); registerMXBean(); @@ -1398,10 +1394,7 @@ public void stop() { } // TODO:Also stop this thread if an OM switches from leader to follower. // Should be fixed after HDDS-4451. - if (this.emptier != null) { - emptier.interrupt(); - emptier = null; - } + stopTrashEmptier(); metadataManager.stop(); metrics.unRegister(); omClientProtocolMetrics.unregister(); @@ -3281,6 +3274,7 @@ TermIndex installCheckpoint(String leaderId, Path checkpointLocation, // During stopServices, if KeyManager was stopped successfully and // OMMetadataManager stop failed, we should restart the KeyManager. keyManager.start(configuration); + startTrashEmptier(configuration); throw e; } @@ -3371,6 +3365,14 @@ void stopServices() throws Exception { keyManager.stop(); stopSecretManager(); metadataManager.stop(); + stopTrashEmptier(); + } + + private void stopTrashEmptier() { + if (this.emptier != null) { + emptier.interrupt(); + emptier = null; + } } /** @@ -3439,6 +3441,7 @@ void reloadOMState(long newSnapshotIndex, long newSnapshotTermIndex) // Restart required services metadataManager.start(configuration); keyManager.start(configuration); + startTrashEmptier(configuration); // Set metrics and start metrics back ground thread metrics.setNumVolumes(metadataManager.countRowsInTable(metadataManager diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java index d532619a1e15..358074f0ed08 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java @@ -137,7 +137,7 @@ public void run() { try { // sleep for interval Thread.sleep(end - now); if (!om.isLeader()){ - return; + continue; } } catch (InterruptedException e) { break; // exit on interrupt From b04abb496ba7c2d22c841148679c97d466f397bc Mon Sep 17 00:00:00 2001 From: sadanand48 Date: Fri, 11 Dec 2020 11:06:33 +0530 Subject: [PATCH 4/7] trigger new CI check From 694aa7ff093c55d33ccfe42d567b15ff1485424c Mon Sep 17 00:00:00 2001 From: sadanand48 Date: Fri, 11 Dec 2020 11:28:39 +0530 Subject: [PATCH 5/7] trigger new CI check From 7e74d15722a3b8a747485606e19568c36b2b39b6 Mon Sep 17 00:00:00 2001 From: sadanand48 Date: Fri, 11 Dec 2020 12:08:51 +0530 Subject: [PATCH 6/7] resolved conflict --- .../java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java index 358074f0ed08..91ad4afc0a71 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java @@ -127,7 +127,7 @@ protected class Emptier implements Runnable { @Override public void run() { // if this is not the leader node,don't run the emptier - if (emptierInterval == 0 || !om.isLeader()) { + if (emptierInterval == 0 || !om.isLeaderReady()) { return; // trash disabled } long now, end; @@ -136,7 +136,7 @@ public void run() { end = ceiling(now, emptierInterval); try { // sleep for interval Thread.sleep(end - now); - if (!om.isLeader()){ + if (!om.isLeaderReady()){ continue; } } catch (InterruptedException e) { From 68d35f3da2d28e360bcc8e781b520ae954be9320 Mon Sep 17 00:00:00 2001 From: Sadanand Shenoy Date: Fri, 11 Dec 2020 14:44:34 +0530 Subject: [PATCH 7/7] Update OzoneManager.java --- .../src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 037cea46e8e9..1b4fc47dc241 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1392,8 +1392,6 @@ public void stop() { if (httpServer != null) { httpServer.stop(); } - // TODO:Also stop this thread if an OM switches from leader to follower. - // Should be fixed after HDDS-4451. stopTrashEmptier(); metadataManager.stop(); metrics.unRegister();