diff --git a/core/common/src/main/java/alluxio/underfs/ObjectUnderFileSystem.java b/core/common/src/main/java/alluxio/underfs/ObjectUnderFileSystem.java index e3dc7513d488..564271d38014 100755 --- a/core/common/src/main/java/alluxio/underfs/ObjectUnderFileSystem.java +++ b/core/common/src/main/java/alluxio/underfs/ObjectUnderFileSystem.java @@ -101,27 +101,26 @@ protected ObjectUnderFileSystem(AlluxioURI uri, UnderFileSystemConfiguration ufs * Information about a single object in object UFS. */ protected class ObjectStatus { - private static final String INVALID_CONTENT_HASH = ""; private static final long INVALID_CONTENT_LENGTH = -1L; - private static final long INVALID_MODIFIED_TIME = -1L; private final String mContentHash; private final long mContentLength; - private final long mLastModifiedTimeMs; + /** Last modified epoch time in ms, or null if it is not available. */ + private final Long mLastModifiedTimeMs; private final String mName; public ObjectStatus(String name, String contentHash, long contentLength, - long lastModifiedTimeMs) { - mContentHash = contentHash; + @Nullable Long lastModifiedTimeMs) { + mContentHash = contentHash == null ? UfsFileStatus.INVALID_CONTENT_HASH : contentHash; mContentLength = contentLength; mLastModifiedTimeMs = lastModifiedTimeMs; mName = name; } public ObjectStatus(String name) { - mContentHash = INVALID_CONTENT_HASH; + mContentHash = UfsFileStatus.INVALID_CONTENT_HASH; mContentLength = INVALID_CONTENT_LENGTH; - mLastModifiedTimeMs = INVALID_MODIFIED_TIME; + mLastModifiedTimeMs = null; mName = name; } @@ -146,7 +145,8 @@ public long getContentLength() { * * @return modification time in milliseconds */ - public long getLastModifiedTimeMs() { + @Nullable + public Long getLastModifiedTimeMs() { return mLastModifiedTimeMs; } diff --git a/core/common/src/main/java/alluxio/underfs/UfsFileStatus.java b/core/common/src/main/java/alluxio/underfs/UfsFileStatus.java index 4ef6c00b880c..24ac03a2536c 100644 --- a/core/common/src/main/java/alluxio/underfs/UfsFileStatus.java +++ b/core/common/src/main/java/alluxio/underfs/UfsFileStatus.java @@ -40,8 +40,9 @@ public class UfsFileStatus extends UfsStatus { * @param xAttr extended attributes, if any * @param blockSize blocksize, -1 if unknown */ - public UfsFileStatus(String name, String contentHash, long contentLength, long lastModifiedTimeMs, - String owner, String group, short mode, @Nullable Map xAttr, long blockSize) { + public UfsFileStatus(String name, String contentHash, long contentLength, + @Nullable Long lastModifiedTimeMs, String owner, String group, short mode, + @Nullable Map xAttr, long blockSize) { super(name, false, owner, group, mode, lastModifiedTimeMs, xAttr); mContentHash = contentHash; mContentLength = contentLength; @@ -60,8 +61,9 @@ public UfsFileStatus(String name, String contentHash, long contentLength, long l * @param mode of the file * @param blockSize blocksize, -1 if unknown */ - public UfsFileStatus(String name, String contentHash, long contentLength, long lastModifiedTimeMs, - String owner, String group, short mode, long blockSize) { + public UfsFileStatus(String name, String contentHash, long contentLength, + @Nullable Long lastModifiedTimeMs, String owner, String group, + short mode, long blockSize) { super(name, false, owner, group, mode, lastModifiedTimeMs, /* xattrs */ null); mContentHash = contentHash; mContentLength = contentLength; @@ -84,8 +86,9 @@ public UfsFileStatus(String name, String contentHash, long contentLength, long l * @param xAttr extended attributes, if any */ @Deprecated - public UfsFileStatus(String name, String contentHash, long contentLength, long lastModifiedTimeMs, - String owner, String group, short mode, @Nullable Map xAttr) { + public UfsFileStatus(String name, String contentHash, long contentLength, + @Nullable Long lastModifiedTimeMs, String owner, String group, short mode, + @Nullable Map xAttr) { this(name, contentHash, contentLength, lastModifiedTimeMs, owner, group, mode, xAttr, UNKNOWN_BLOCK_SIZE); } @@ -105,8 +108,8 @@ public UfsFileStatus(String name, String contentHash, long contentLength, long l * @param mode of the file */ @Deprecated - public UfsFileStatus(String name, String contentHash, long contentLength, long lastModifiedTimeMs, - String owner, String group, short mode) { + public UfsFileStatus(String name, String contentHash, long contentLength, + @Nullable Long lastModifiedTimeMs, String owner, String group, short mode) { this(name, contentHash, contentLength, lastModifiedTimeMs, owner, group, mode, null, UNKNOWN_BLOCK_SIZE); } diff --git a/core/common/src/main/java/alluxio/underfs/UfsStatus.java b/core/common/src/main/java/alluxio/underfs/UfsStatus.java index 73f0300a4d54..2314dce8cab9 100644 --- a/core/common/src/main/java/alluxio/underfs/UfsStatus.java +++ b/core/common/src/main/java/alluxio/underfs/UfsStatus.java @@ -49,7 +49,7 @@ public abstract class UfsStatus { * @param xAttrs any extended attributes on the inode */ protected UfsStatus(String name, boolean isDirectory, String owner, String group, short mode, - Long lastModifiedTimeMs, @Nullable Map xAttrs) { + @Nullable Long lastModifiedTimeMs, @Nullable Map xAttrs) { mIsDirectory = isDirectory; mName = name; mOwner = owner; diff --git a/core/server/common/src/main/java/alluxio/master/journal/ufs/UfsJournalGarbageCollector.java b/core/server/common/src/main/java/alluxio/master/journal/ufs/UfsJournalGarbageCollector.java index b2e1b2b26fde..a20cd2bf53d3 100644 --- a/core/server/common/src/main/java/alluxio/master/journal/ufs/UfsJournalGarbageCollector.java +++ b/core/server/common/src/main/java/alluxio/master/journal/ufs/UfsJournalGarbageCollector.java @@ -115,13 +115,17 @@ private void gcFileIfStale(UfsJournalFile file, long checkpointSequenceNumber) { return; } - long lastModifiedTimeMs; + Long lastModifiedTimeMs; try { lastModifiedTimeMs = mUfs.getFileStatus(file.getLocation().toString()).getLastModifiedTime(); } catch (IOException e) { LOG.warn("Failed to get the last modified time for {}.", file.getLocation()); return; } + if (lastModifiedTimeMs == null) { + LOG.warn("Failed to get the last modified time for {}.", file.getLocation()); + return; + } long thresholdMs = file.isTmpCheckpoint() ? ServerConfiguration.getMs(PropertyKey.MASTER_JOURNAL_TEMPORARY_FILE_GC_THRESHOLD_MS) diff --git a/core/server/common/src/main/java/alluxio/master/journalv0/ufs/UfsJournalReader.java b/core/server/common/src/main/java/alluxio/master/journalv0/ufs/UfsJournalReader.java index 8d01bd856e0a..4d11c588d10b 100644 --- a/core/server/common/src/main/java/alluxio/master/journalv0/ufs/UfsJournalReader.java +++ b/core/server/common/src/main/java/alluxio/master/journalv0/ufs/UfsJournalReader.java @@ -107,7 +107,12 @@ public long getCheckpointLastModifiedTimeMs() throws IOException { if (!mUfs.isFile(mCheckpoint.toString())) { throw new IOException("Checkpoint file " + mCheckpoint + " does not exist."); } - mCheckpointLastModifiedTime = mUfs.getFileStatus(mCheckpoint.toString()).getLastModifiedTime(); + Long lastModifiedTime = mUfs.getFileStatus(mCheckpoint.toString()).getLastModifiedTime(); + if (lastModifiedTime == null) { + throw new IOException("Failed to get checkpoint file " + + mCheckpoint + " last modified time."); + } + mCheckpointLastModifiedTime = lastModifiedTime; return mCheckpointLastModifiedTime; } } diff --git a/core/server/master/src/test/java/alluxio/master/file/FileSystemMasterSyncMetadataTest.java b/core/server/master/src/test/java/alluxio/master/file/FileSystemMasterSyncMetadataTest.java index c2eec6e67229..69d3de8b4319 100644 --- a/core/server/master/src/test/java/alluxio/master/file/FileSystemMasterSyncMetadataTest.java +++ b/core/server/master/src/test/java/alluxio/master/file/FileSystemMasterSyncMetadataTest.java @@ -162,7 +162,7 @@ public void listStatusWithSyncMetadataAndEmptyS3Owner() throws Exception { // Mock nested ufs path AlluxioURI nestedFilePath = ufsMount.join("dir1").join("file1"); UfsFileStatus nestedFileStatus = new UfsFileStatus(nestedFilePath.getPath(), "dummy", 0, - 0, "", "", mode, 1024); + null, "", "", mode, 1024); Mockito.when(mUfs.getFingerprint(nestedFilePath.toString())) .thenReturn(Fingerprint.create("s3", nestedFileStatus).serialize()); Mockito.when(mUfs.getStatus(nestedFilePath.toString())).thenReturn(nestedFileStatus); @@ -178,7 +178,7 @@ public void listStatusWithSyncMetadataAndEmptyS3Owner() throws Exception { // Mock creating the same directory and nested file in UFS out of band Mockito.when(mUfs.listStatus(eq(dir1Path.toString()))) .thenReturn(new UfsStatus[]{new UfsFileStatus("file1", "dummy", 0, - 0, "", "", mode, 1024)}); + null, "", "", mode, 1024)}); // List with sync.interval=0 List fileInfoList = diff --git a/underfs/gcs/src/main/java/alluxio/underfs/gcs/GCSUnderFileSystem.java b/underfs/gcs/src/main/java/alluxio/underfs/gcs/GCSUnderFileSystem.java index 59c4c4bcf2f9..20b4767dccbd 100644 --- a/underfs/gcs/src/main/java/alluxio/underfs/gcs/GCSUnderFileSystem.java +++ b/underfs/gcs/src/main/java/alluxio/underfs/gcs/GCSUnderFileSystem.java @@ -45,6 +45,7 @@ import java.io.OutputStream; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.util.Date; import java.util.function.Supplier; import javax.annotation.concurrent.ThreadSafe; @@ -255,8 +256,10 @@ public ObjectStatus[] getObjectStatuses() { StorageObject[] objects = mChunk.getObjects(); ObjectStatus[] ret = new ObjectStatus[objects.length]; for (int i = 0; i < ret.length; ++i) { + Date lastModifiedDate = objects[i].getLastModifiedDate(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); ret[i] = new ObjectStatus(objects[i].getKey(), objects[i].getMd5HashAsBase64(), - objects[i].getContentLength(), objects[i].getLastModifiedDate().getTime()); + objects[i].getContentLength(), lastModifiedTime); } return ret; } diff --git a/underfs/gcs/src/main/java/alluxio/underfs/gcs/v2/GCSV2UnderFileSystem.java b/underfs/gcs/src/main/java/alluxio/underfs/gcs/v2/GCSV2UnderFileSystem.java index 5a723b8059c8..ddd24eb4cdb9 100644 --- a/underfs/gcs/src/main/java/alluxio/underfs/gcs/v2/GCSV2UnderFileSystem.java +++ b/underfs/gcs/src/main/java/alluxio/underfs/gcs/v2/GCSV2UnderFileSystem.java @@ -306,8 +306,8 @@ protected InputStream openObject(String key, OpenOptions options, * @return the blob status */ private ObjectStatus getBlobStatus(Blob blob) { - long time = blob.getUpdateTime() != null ? blob.getUpdateTime() - : blob.getCreateTime() != null ? blob.getCreateTime() : -1; + Long time = blob.getUpdateTime() != null ? blob.getUpdateTime() + : blob.getCreateTime() != null ? blob.getCreateTime() : null; return new ObjectStatus(blob.getName(), blob.getMd5() == null ? DIR_HASH : blob.getMd5(), blob.getSize(), time); } diff --git a/underfs/obs/src/main/java/alluxio/underfs/obs/OBSUnderFileSystem.java b/underfs/obs/src/main/java/alluxio/underfs/obs/OBSUnderFileSystem.java index c9601fdbb0eb..6ca1a108ab3e 100644 --- a/underfs/obs/src/main/java/alluxio/underfs/obs/OBSUnderFileSystem.java +++ b/underfs/obs/src/main/java/alluxio/underfs/obs/OBSUnderFileSystem.java @@ -38,6 +38,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.Date; import java.util.List; import javax.annotation.concurrent.ThreadSafe; @@ -244,8 +245,11 @@ public ObjectStatus[] getObjectStatuses() { ObjectStatus[] ret = new ObjectStatus[objects.size()]; int i = 0; for (ObsObject obj : objects) { - ret[i++] = new ObjectStatus(obj.getObjectKey(), obj.getMetadata().getEtag(), - obj.getMetadata().getContentLength(), obj.getMetadata().getLastModified().getTime()); + ObjectMetadata meta = obj.getMetadata(); + Date lastModifiedDate = meta.getLastModified(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); + ret[i++] = new ObjectStatus(obj.getObjectKey(), meta.getEtag(), + meta.getContentLength(), lastModifiedTime); } return ret; } @@ -291,8 +295,10 @@ protected ObjectStatus getObjectStatus(String key) { return null; } } + Date lastModifiedDate = meta.getLastModified(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); return new ObjectStatus(key, meta.getEtag(), meta.getContentLength(), - meta.getLastModified().getTime()); + lastModifiedTime); } catch (ObsException e) { LOG.warn("Failed to get Object {}, return null", key, e); return null; diff --git a/underfs/obs/src/test/java/alluxio/underfs/obs/OBSUnderFileSystemTest.java b/underfs/obs/src/test/java/alluxio/underfs/obs/OBSUnderFileSystemTest.java index 148ddb14c63e..08ffc6769c2e 100644 --- a/underfs/obs/src/test/java/alluxio/underfs/obs/OBSUnderFileSystemTest.java +++ b/underfs/obs/src/test/java/alluxio/underfs/obs/OBSUnderFileSystemTest.java @@ -141,4 +141,26 @@ public void judgeDirectoryInBucket() throws Exception { Assert.assertTrue(mOBSUnderFileSystem.isDirectory("dir1")); Assert.assertFalse(mOBSUnderFileSystem.isDirectory("obs_file1")); } + + @Test + public void nullObjectMetaTest() throws Exception { + ObjectMetadata fileMeta = new ObjectMetadata(); + fileMeta.setContentLength(10L); + ObjectMetadata dirMeta = new ObjectMetadata(); + dirMeta.setContentLength(0L); + /** + * /xx/file1/ ( File1 actually exists, which is a file) , there is / after file1 name. + * When OBS, the path object meta is null. + * When PFS, the path object meta is not null. The object meta is same as /xx/file1 + */ + Mockito.when(mClient.getObjectMetadata(BUCKET_NAME, "pfs_file1")) + .thenReturn(fileMeta); + Mockito.when(mClient.getObjectMetadata(BUCKET_NAME, "dir1")) + .thenReturn(dirMeta); + + mOBSUnderFileSystem = new OBSUnderFileSystem(new AlluxioURI(""), mClient, BUCKET_NAME, "obs", + UnderFileSystemConfiguration.defaults(ConfigurationTestUtils.defaults())); + Assert.assertNotNull(mOBSUnderFileSystem.getObjectStatus("pfs_file1")); + Assert.assertNotNull(mOBSUnderFileSystem.getObjectStatus("dir1")); + } } diff --git a/underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystem.java b/underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystem.java index 409ed2c0593d..46afd3cf0dcc 100644 --- a/underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystem.java +++ b/underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystem.java @@ -39,6 +39,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.Date; import java.util.List; import javax.annotation.concurrent.ThreadSafe; @@ -211,8 +212,10 @@ public ObjectStatus[] getObjectStatuses() { ObjectStatus[] ret = new ObjectStatus[objects.size()]; int i = 0; for (OSSObjectSummary obj : objects) { + Date lastModifiedDate = obj.getLastModified(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); ret[i++] = new ObjectStatus(obj.getKey(), obj.getETag(), obj.getSize(), - obj.getLastModified().getTime()); + lastModifiedTime); } return ret; } @@ -243,8 +246,10 @@ protected ObjectStatus getObjectStatus(String key) { if (meta == null) { return null; } + Date lastModifiedDate = meta.getLastModified(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); return new ObjectStatus(key, meta.getETag(), meta.getContentLength(), - meta.getLastModified().getTime()); + lastModifiedTime); } catch (ServiceException e) { return null; } diff --git a/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java b/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java index e2f6f9982696..e251fa0926d9 100644 --- a/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java +++ b/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java @@ -536,8 +536,10 @@ public ObjectStatus[] getObjectStatuses() { ObjectStatus[] ret = new ObjectStatus[objects.size()]; int i = 0; for (S3ObjectSummary obj : objects) { + Date lastModifiedDate = obj.getLastModified(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); ret[i++] = new ObjectStatus(obj.getKey(), obj.getETag(), obj.getSize(), - obj.getLastModified().getTime()); + lastModifiedTime); } return ret; } @@ -580,8 +582,10 @@ public ObjectStatus[] getObjectStatuses() { ObjectStatus[] ret = new ObjectStatus[objects.size()]; int i = 0; for (S3ObjectSummary obj : objects) { + Date lastModifiedDate = obj.getLastModified(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); ret[i++] = new ObjectStatus(obj.getKey(), obj.getETag(), obj.getSize(), - obj.getLastModified().getTime()); + lastModifiedTime); } return ret; } @@ -610,8 +614,9 @@ public ObjectListingChunk getNextChunk() throws IOException { protected ObjectStatus getObjectStatus(String key) throws IOException { try { ObjectMetadata meta = mClient.getObjectMetadata(mBucketName, key); - return new ObjectStatus(key, meta.getETag(), meta.getContentLength(), - meta.getLastModified().getTime()); + Date lastModifiedDate = meta.getLastModified(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); + return new ObjectStatus(key, meta.getETag(), meta.getContentLength(), lastModifiedTime); } catch (AmazonServiceException e) { if (e.getStatusCode() == 404) { // file not found, possible for exists calls return null; diff --git a/underfs/s3a/src/test/java/alluxio/underfs/s3a/S3AUnderFileSystemTest.java b/underfs/s3a/src/test/java/alluxio/underfs/s3a/S3AUnderFileSystemTest.java index 832d4cccf0fe..948e85604be7 100644 --- a/underfs/s3a/src/test/java/alluxio/underfs/s3a/S3AUnderFileSystemTest.java +++ b/underfs/s3a/src/test/java/alluxio/underfs/s3a/S3AUnderFileSystemTest.java @@ -29,6 +29,7 @@ import com.amazonaws.services.s3.AmazonS3Client; import com.amazonaws.services.s3.model.AccessControlList; import com.amazonaws.services.s3.model.ListObjectsV2Request; +import com.amazonaws.services.s3.model.ObjectMetadata; import com.amazonaws.services.s3.model.Owner; import com.amazonaws.services.s3.transfer.TransferManager; import com.google.common.util.concurrent.ListeningExecutorService; @@ -253,4 +254,12 @@ public void stripPrefixIfPresent() throws Exception { Assert.assertEquals("", mS3UnderFileSystem.stripPrefixIfPresent("")); Assert.assertEquals("", mS3UnderFileSystem.stripPrefixIfPresent("/")); } + + @Test + public void getNullLastModifiedTime() throws IOException { + Mockito.when(mClient.getObjectMetadata(Matchers.anyString(), Matchers.anyString())) + .thenReturn(new ObjectMetadata()); + // throw NPE before https://github.com/Alluxio/alluxio/pull/14641 + mS3UnderFileSystem.getObjectStatus(PATH); + } } diff --git a/underfs/swift/src/main/java/alluxio/underfs/swift/SwiftUnderFileSystem.java b/underfs/swift/src/main/java/alluxio/underfs/swift/SwiftUnderFileSystem.java index e60f8472ddf4..41292cb1dbf5 100644 --- a/underfs/swift/src/main/java/alluxio/underfs/swift/SwiftUnderFileSystem.java +++ b/underfs/swift/src/main/java/alluxio/underfs/swift/SwiftUnderFileSystem.java @@ -45,6 +45,7 @@ import java.io.OutputStream; import java.util.ArrayDeque; import java.util.Arrays; +import java.util.Date; import java.util.List; import javax.annotation.concurrent.ThreadSafe; @@ -313,9 +314,11 @@ public ObjectStatus[] getObjectStatuses() { ObjectStatus[] res = new ObjectStatus[objects.size()]; for (DirectoryOrObject object : objects) { if (object.isObject()) { + Date lastModifiedDate = object.getAsObject().getLastModifiedAsDate(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); res[i++] = new ObjectStatus(object.getName(), object.getAsObject().getEtag(), object.getAsObject().getContentLength(), - object.getAsObject().getLastModifiedAsDate().getTime()); + lastModifiedTime); } else { res[i++] = new ObjectStatus(object.getName()); } @@ -345,8 +348,10 @@ protected ObjectStatus getObjectStatus(String key) { Container container = mAccount.getContainer(mContainerName); StoredObject meta = container.getObject(key); if (meta != null && meta.exists()) { + Date lastModifiedDate = meta.getLastModifiedAsDate(); + Long lastModifiedTime = lastModifiedDate == null ? null : lastModifiedDate.getTime(); return new ObjectStatus(key, meta.getEtag(), meta.getContentLength(), - meta.getLastModifiedAsDate().getTime()); + lastModifiedTime); } return null; }