bgaborg commented on a change in pull request #802: HADOOP-16279. S3Guard:
Implement time-based (TTL) expiry for entries …
URL: https://github.com/apache/hadoop/pull/802#discussion_r291191731
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardOutOfBandOperations.java
##########
@@ -271,6 +281,202 @@ public void testListingDelete() throws Exception {
deleteFileInListing();
}
+ /**
+ * Tests that tombstone expiry is implemented, so if a file is created raw
+ * while the tombstone exist in ms for with the same name then S3Guard will
+ * check S3 for the file.
+ *
+ * Seq: create guarded; delete guarded; create raw (same path); read guarded;
+ * This will fail if no tombstone expiry is set
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testTombstoneExpiryGuardedDeleteRawCreate() throws Exception {
+ boolean allowAuthoritative = authoritative;
+ Path testFilePath = path("TEGDRC-" + UUID.randomUUID());
+ LOG.info("Allow authoritative param: {}", allowAuthoritative);
+ String originalText = "some test";
+ String newText = "the new originalText for test";
+
+ final ITtlTimeProvider originalTimeProvider =
+ guardedFs.getTtlTimeProvider();
+ try {
+ final AtomicLong now = new AtomicLong(1);
+ final AtomicLong metadataTtl = new AtomicLong(1);
+
+ // SET TTL TIME PROVIDER FOR TESTING
+ ITtlTimeProvider testTimeProvider =
+ new ITtlTimeProvider() {
+ @Override public long getNow() {
+ return now.get();
+ }
+
+ @Override public long getMetadataTtl() {
+ return metadataTtl.get();
+ }
+ };
+ guardedFs.setTtlTimeProvider(testTimeProvider);
+
+ // CREATE GUARDED
+ createAndAwaitFs(guardedFs, testFilePath, originalText);
+
+ // DELETE GUARDED
+ deleteGuardedTombstoned(guardedFs, testFilePath, now);
+
+ // CREATE RAW
+ createAndAwaitFs(rawFS, testFilePath, newText);
+
+ // CHECK LISTING - THE FILE SHOULD NOT BE THERE, EVEN IF IT'S CREATED RAW
+ checkListingDoesNotContainPath(guardedFs, testFilePath);
+
+ // CHANGE TTL SO ENTRY (& TOMBSTONE METADATA) WILL EXPIRE
+ long willExpire = now.get() + metadataTtl.get() + 1L;
+ now.set(willExpire);
+ LOG.info("willExpire: {}, ttlNow: {}; ttlTTL: {}", willExpire,
+ testTimeProvider.getNow(), testTimeProvider.getMetadataTtl());
+
+ // READ GUARDED
+ String newRead = readBytesToString(guardedFs, testFilePath,
+ newText.length());
+
+ // CHECK LISTING - THE FILE SHOULD BE THERE, TOMBSTONE EXPIRED
+ checkListingContainsPath(guardedFs, testFilePath);
+
+ // we can assert that the originalText is the new one, which created raw
+ LOG.info("Old: {}, New: {}, Read: {}", originalText, newText, newRead);
+ assertEquals("The text should be modified with a new.", newText,
+ newRead);
+ } finally {
+ guardedFs.delete(testFilePath, true);
+ guardedFs.setTtlTimeProvider(originalTimeProvider);
+ }
+ }
+
+ private void createAndAwaitFs(S3AFileSystem fs, Path testFilePath,
+ String text) throws Exception {
+ writeTextFile(fs, testFilePath, text, true);
+ final FileStatus newStatus = awaitFileStatus(fs, testFilePath);
+ assertNotNull("Newly created file status should not be null.", newStatus);
+ }
+
+ private void deleteGuardedTombstoned(S3AFileSystem guardedFs,
+ Path testFilePath, AtomicLong now) throws Exception {
+ guardedFs.delete(testFilePath, true);
+
+ final PathMetadata metadata =
+ guardedFs.getMetadataStore().get(testFilePath);
+ assertNotNull("Created file metadata should not be null in ms",
+ metadata);
+ assertEquals("Created file metadata last_updated should equal with "
+ + "mocked now", now.get(), metadata.getLastUpdated());
+
+ intercept(FileNotFoundException.class, testFilePath.toString(),
+ "This file should throw FNFE when reading through "
+ + "the guarded fs, and the metadatastore tombstoned the file.",
+ () -> guardedFs.getFileStatus(testFilePath));
+ }
+
+ /**
+ * createNonRecursive must fail if the parent directory has been deleted,
+ * and succeed if the tombstone has expired and the directory has been
+ * created out of band.
+ */
+ @Test
+ public void createNonRecursiveFailsIfParentDeleted() throws Exception {
+ LOG.info("Authoritative mode: {}", authoritative);
+
+ String dirToDelete = methodName + UUID.randomUUID().toString();
+ String fileToTry = dirToDelete + "/theFileToTry";
+
+ final Path dirPath = path(dirToDelete);
+ final Path filePath = path(fileToTry);
+
+ // Create a directory with
+ ITtlTimeProvider mockTimeProvider = mock(ITtlTimeProvider.class);
+ ITtlTimeProvider originalTimeProvider = guardedFs.getTtlTimeProvider();
+
+ try {
+ guardedFs.setTtlTimeProvider(mockTimeProvider);
+ when(mockTimeProvider.getNow()).thenReturn(100L);
+ when(mockTimeProvider.getMetadataTtl()).thenReturn(5L);
+
+ // CREATE DIRECTORY
+ guardedFs.mkdirs(dirPath);
+
+ // DELETE DIRECTORY
+ guardedFs.delete(dirPath, true);
+
+ // WRITE TO DELETED DIRECTORY - FAIL
+ intercept(FileNotFoundException.class,
+ dirToDelete,
+ "createNonRecursive must fail if the parent directory has been
deleted.",
+ () -> createNonRecursive(guardedFs, filePath));
+
+ // CREATE THE DIRECTORY RAW
+ rawFS.mkdirs(dirPath);
+ awaitFileStatus(rawFS, dirPath);
+
+ // SET TIME SO METADATA EXPIRES
+ when(mockTimeProvider.getNow()).thenReturn(110L);
+
+ // WRITE TO DELETED DIRECTORY - SUCCESS
+ createNonRecursive(guardedFs, filePath);
+
+ } finally {
+ guardedFs.delete(filePath, true);
+ guardedFs.delete(dirPath, true);
+ guardedFs.setTtlTimeProvider(originalTimeProvider);
+ }
+ }
+
+ private void checkListingDoesNotContainPath(S3AFileSystem fs, Path filePath)
+ throws IOException {
+ final RemoteIterator<LocatedFileStatus> listIter =
+ fs.listFiles(filePath.getParent(), false);
+ while (listIter.hasNext()) {
+ final LocatedFileStatus lfs = listIter.next();
+ assertNotEquals("The tombstone has not been expired, so must not be"
+ + " listed.", filePath, lfs.getPath());
+ }
+ LOG.info("{}; file omitted from listFiles listing as expected.", filePath);
+
+ final FileStatus[] fileStatuses = fs.listStatus(filePath.getParent());
+ for (FileStatus fileStatus : fileStatuses) {
+ assertNotEquals("The tombstone has not been expired, so must not be"
+ + " listed.", filePath, fileStatus.getPath());
+ }
+ LOG.info("{}; file omitted from listStatus as expected.", filePath);
+ }
+
+ private void checkListingContainsPath(S3AFileSystem fs, Path filePath)
+ throws IOException {
+ final RemoteIterator<LocatedFileStatus> listIter =
+ fs.listFiles(filePath.getParent(), false);
+
+ boolean lfsHit = false;
+ while (listIter.hasNext()) {
+ final LocatedFileStatus lfs = listIter.next();
+ if (lfs.getPath().equals(filePath)) {
+ lfsHit = true;
+ LOG.info("{}; file found in listFiles as expected.", filePath);
+ break;
+ }
+ }
+ assertTrue("The file should be listed in fs.listFiles: ", lfsHit);
+
+ boolean lsHit = false;
+ final FileStatus[] fileStatuses = fs.listStatus(filePath.getParent());
+ for (FileStatus fileStatus : fileStatuses) {
+ if (fileStatus.getPath().equals(filePath)) {
+ lsHit = true;
Review comment:
using a search in the folder was intended instead of using assertEquals
I wanted to write a reusable util method, but I can use assertEquals if we
assume no other files are in that folder.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
