Skip to content

HDDS-6645. Intermittent timeout in TestOzoneFileSystem#testTrash. #5330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aryangupta1998 merged 4 commits into from
Oct 3, 2023
Merged

HDDS-6645. Intermittent timeout in TestOzoneFileSystem#testTrash. #5330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dc872dd
rebase branch
Sep 25, 2023
0d39e7b
address comments
Sep 28, 2023
37427c8
Merge remote-tracking branch 'apache/master' into HDDS-6645
Sep 28, 2023
bf49eb9
fix conflict after master merge
Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions ...-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.ozone.test.GenericTestUtils;
import org.apache.ozone.test.TestClock;
import org.apache.ozone.test.tag.Flaky;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
Expand Down Expand Up @@ -1635,7 +1634,6 @@ public void testDeleteRootWithTrash() throws IOException {
* 2.Verify that the key gets deleted by the trash emptier.
*/
@Test
@Flaky("HDDS-6645")
public void testTrash() throws Exception {
String testKeyName = "testKey2";
Path path = new Path(OZONE_URI_DELIMITER, testKeyName);
Expand All @@ -1655,12 +1653,10 @@ public void testTrash() throws Exception {

// Call moveToTrash. We can't call protected fs.rename() directly
trash.moveToTrash(path);
// Added this assertion here and will be tested as part of testTrash
// test case which needs to be tested with separate mini cluster having
// emptier thread started with close match of timings of relevant
// assertion statements and corresponding trash and checkpoint interval.

Assert.assertTrue(o3fs.exists(userTrash));
Assert.assertTrue(o3fs.exists(userTrashCurrent));
Assert.assertTrue(o3fs.exists(trashPath) || o3fs.listStatus(
o3fs.listStatus(userTrash)[0].getPath()).length > 0);

// Wait until the TrashEmptier purges the key
GenericTestUtils.waitFor(() -> {
Expand All @@ -1673,10 +1669,6 @@ public void testTrash() throws Exception {
}
}, 100, 120000);

// userTrash path will contain the checkpoint folder
FileStatus[] statusList = fs.listStatus(userTrash);
Assert.assertNotEquals(Arrays.toString(statusList), 0, statusList.length);

// wait for deletion of checkpoint dir
GenericTestUtils.waitFor(() -> {
try {
Expand Down
67 changes: 44 additions & 23 deletions ...op-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
import org.apache.hadoop.fs.FSDataOutputStream;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.hadoop.hdds.utils.db.TableIterator;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
import org.apache.hadoop.ozone.OFSPath;
Expand All @@ -49,7 +47,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.Closeable;
import java.io.IOException;
import java.net.InetAddress;
import java.net.URI;
Expand All @@ -59,6 +56,7 @@
import java.util.Map;
import java.util.Iterator;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;

import static org.apache.hadoop.ozone.OzoneConsts.OZONE_O3TRASH_URI_SCHEME;
import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER;
Expand All @@ -75,6 +73,8 @@ public class TrashOzoneFileSystem extends FileSystem {

private static final int OZONE_FS_ITERATE_BATCH_SIZE = 100;

private static final int OZONE_MAX_LIST_KEYS_SIZE = 10000;

private final OzoneManager ozoneManager;

private final String userName;
Expand Down Expand Up @@ -167,9 +167,8 @@ public boolean rename(Path src, Path dst) throws IOException {
equals(dstPath.getBucketName()));
Preconditions.checkArgument(srcPath.getTrashRoot().
toString().equals(dstPath.getTrashRoot().toString()));
try (RenameIterator iterator = new RenameIterator(src, dst)) {
iterator.iterate();
}
RenameIterator iterator = new RenameIterator(src, dst);
iterator.iterate();
return true;
}

Expand Down Expand Up @@ -198,9 +197,8 @@ public boolean delete(Path path, boolean b) throws IOException {
if (bucket.getBucketLayout().isFileSystemOptimized()) {
return deleteFSO(srcPath);
}
try (DeleteIterator iterator = new DeleteIterator(path, true)) {
iterator.iterate();
}
DeleteIterator iterator = new DeleteIterator(path, true);
iterator.iterate();
return true;
}

Expand Down Expand Up @@ -347,12 +345,11 @@ public boolean exists(Path f) throws IOException {
}
}

private abstract class OzoneListingIterator implements Closeable {
private abstract class OzoneListingIterator {
private final Path path;
private final FileStatus status;
private String pathKey;
private TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
keyIterator;
private Iterator<String> keyIterator;

OzoneListingIterator(Path path)
throws IOException {
Expand All @@ -362,10 +359,30 @@ private abstract class OzoneListingIterator implements Closeable {
if (status.isDirectory()) {
this.pathKey = addTrailingSlashIfNeeded(pathKey);
}
keyIterator = ozoneManager.getMetadataManager().getKeyIterator();
OFSPath fsPath = new OFSPath(pathKey,
OzoneConfiguration.of(getConf()));
keyIterator =
getKeyIterator(fsPath.getVolumeName(), fsPath.getBucketName(),
fsPath.getKeyName());
}

/**
private Iterator<String> getKeyIterator(String volumeName,
String bucketName, String keyName) throws IOException {
List<String> keys = new ArrayList<>(
listKeys(volumeName, bucketName, "", keyName));
String lastKey = keys.get(keys.size() - 1);
List<String> nextBatchKeys =
listKeys(volumeName, bucketName, lastKey, keyName);

while (!nextBatchKeys.isEmpty()) {
keys.addAll(nextBatchKeys);
lastKey = nextBatchKeys.get(nextBatchKeys.size() - 1);
nextBatchKeys = listKeys(volumeName, bucketName, lastKey, keyName);
}
return keys.iterator();
}

/**
* The output of processKey determines if further iteration through the
* keys should be done or not.
*
Expand Down Expand Up @@ -395,13 +412,10 @@ boolean iterate() throws IOException {
String ofsPathprefix =
ofsPath.getNonKeyPathNoPrefixDelim() + OZONE_URI_DELIMITER;
while (keyIterator.hasNext()) {
Table.KeyValue< String, OmKeyInfo > kv = keyIterator.next();
String keyPath = ofsPathprefix + kv.getValue().getKeyName();
String keyName = keyIterator.next();
String keyPath = ofsPathprefix + keyName;
LOG.trace("iterating key path: {}", keyPath);
if (!kv.getValue().getKeyName().equals("")
&& kv.getKey().startsWith("/" + pathKey)) {
keyPathList.add(keyPath);
}
keyPathList.add(keyPath);
if (keyPathList.size() >= OZONE_FS_ITERATE_BATCH_SIZE) {
if (!processKeyPath(keyPathList)) {
return false;
Expand All @@ -427,9 +441,16 @@ FileStatus getStatus() {
return status;
}

@Override
public void close() throws IOException {
keyIterator.close();
/**
* Return a listKeys output with only a list of keyNames.
*/
List<String> listKeys(String volumeName, String bucketName, String startKey,
String keyPrefix) throws IOException {
OMMetadataManager metadataManager = ozoneManager.getMetadataManager();
return metadataManager.listKeys(volumeName, bucketName, startKey,
keyPrefix, OZONE_MAX_LIST_KEYS_SIZE).getKeys().stream()
.map(OmKeyInfo::getKeyName)
.collect(Collectors.toList());
}
}

Expand Down
28 changes: 17 additions & 11 deletions hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,17 +322,8 @@ public void run() {
continue;
}
TrashPolicyOzone trash = new TrashPolicyOzone(fs, conf, om);
Runnable task = () -> {
try {
om.getMetrics().incNumTrashRootsProcessed();
trash.deleteCheckpoint(trashRoot.getPath(), false);
trash.createCheckpoint(trashRoot.getPath(),
new Date(Time.now()));
} catch (Exception e) {
om.getMetrics().incNumTrashFails();
LOG.error("Unable to checkpoint:" + trashRoot.getPath(), e);
}
};
Path trashRootPath = trashRoot.getPath();
Runnable task = getEmptierTask(trashRootPath, trash, false);
om.getMetrics().incNumTrashRootsEnqueued();
executor.submit(task);
}
Expand All @@ -357,6 +348,21 @@ public void run() {
}
}

private Runnable getEmptierTask(Path trashRootPath, TrashPolicyOzone trash,
boolean deleteImmediately) {
Runnable task = () -> {
try {
om.getMetrics().incNumTrashRootsProcessed();
trash.deleteCheckpoint(trashRootPath, deleteImmediately);
trash.createCheckpoint(trashRootPath, new Date(Time.now()));
} catch (Exception e) {
om.getMetrics().incNumTrashFails();
LOG.error("Unable to checkpoint:" + trashRootPath, e);
}
};
return task;
}

private long ceiling(long time, long interval) {
return floor(time, interval) + interval;
}
Expand Down