diff --git a/core/server/master/src/main/java/alluxio/master/file/InodeSyncStream.java b/core/server/master/src/main/java/alluxio/master/file/InodeSyncStream.java index d6fbe3c75ebb..b3a78818c0a3 100644 --- a/core/server/master/src/main/java/alluxio/master/file/InodeSyncStream.java +++ b/core/server/master/src/main/java/alluxio/master/file/InodeSyncStream.java @@ -48,6 +48,7 @@ import alluxio.master.file.meta.LockingScheme; import alluxio.master.file.meta.MountTable; import alluxio.master.file.meta.MutableInodeFile; +import alluxio.master.file.meta.SyncCheck; import alluxio.master.file.meta.SyncCheck.SyncResult; import alluxio.master.file.meta.UfsAbsentPathCache; import alluxio.master.file.meta.UfsSyncPathCache; @@ -435,7 +436,7 @@ private SyncStatus syncInternal() throws DefaultFileSystemMaster.Metrics.INODE_SYNC_STREAM_SKIPPED.inc(); return SyncStatus.NOT_NEEDED; } - if (mDedupConcurrentSync) { + if (mDedupConcurrentSync && mRootScheme.shouldSync() != SyncCheck.SHOULD_SYNC) { /* * If a concurrent sync on the same path is successful after this sync had already * been initialized and that sync is successful, then there is no need to sync again. @@ -456,9 +457,10 @@ private SyncStatus syncInternal() throws * Note that this still applies if A is to sync recursively path /aaa while B is to * sync path /aaa/bbb as the sync scope of A covers B's. */ - boolean shouldSync = mUfsSyncPathCache.shouldSyncPath(mRootScheme.getPath(), mSyncInterval, + boolean shouldSkipSync = + mUfsSyncPathCache.shouldSyncPath(mRootScheme.getPath(), mSyncInterval, mDescendantType).getLastSyncTime() > mRootScheme.shouldSync().getLastSyncTime(); - if (shouldSync) { + if (shouldSkipSync) { DefaultFileSystemMaster.Metrics.INODE_SYNC_STREAM_SKIPPED.inc(); LOG.debug("Skipped sync on {} due to successful concurrent sync", mRootScheme.getPath()); return SyncStatus.NOT_NEEDED; diff --git a/core/server/master/src/main/java/alluxio/master/file/meta/LockingScheme.java b/core/server/master/src/main/java/alluxio/master/file/meta/LockingScheme.java index 07f64136803d..8cacbe364cf6 100644 --- a/core/server/master/src/main/java/alluxio/master/file/meta/LockingScheme.java +++ b/core/server/master/src/main/java/alluxio/master/file/meta/LockingScheme.java @@ -32,13 +32,19 @@ public final class LockingScheme { private final LockPattern mDesiredLockPattern; private final SyncCheck mShouldSync; + // CHECKSTYLE.OFF: LineLengthExceed - cannot break the method link /** * Constructs a {@link LockingScheme}. * + * Avoid using this constructor where shouldSync is set true, if possible. + * {@link #LockingScheme(AlluxioURI, LockPattern, FileSystemMasterCommonPOptions, UfsSyncPathCache, DescendantType)} + * is the preferred one in such case, to make the metadata sync dedup feature work. + * * @param path the path to lock * @param desiredLockPattern the desired lock mode * @param shouldSync true if the path should be synced */ + // CHECKSTYLE.ON: LineLengthExceed public LockingScheme(AlluxioURI path, LockPattern desiredLockPattern, boolean shouldSync) { mPath = path; mDesiredLockPattern = desiredLockPattern; diff --git a/core/server/master/src/test/java/alluxio/master/file/FileSystemMasterSyncMetadataConcurrentTest.java b/core/server/master/src/test/java/alluxio/master/file/FileSystemMasterSyncMetadataConcurrentTest.java index 1ea48f3bffbe..824f65184c92 100644 --- a/core/server/master/src/test/java/alluxio/master/file/FileSystemMasterSyncMetadataConcurrentTest.java +++ b/core/server/master/src/test/java/alluxio/master/file/FileSystemMasterSyncMetadataConcurrentTest.java @@ -162,6 +162,23 @@ public void syncTheSameDirectoryButTheSecondCallCancelled() throws Exception { assertEquals(InodeSyncStream.SyncStatus.OK, iss3.sync()); } + @Test + public void syncWhenShouldSyncIsSetTrue() throws Exception { + Supplier inodeSyncStreamSupplier = () -> new InodeSyncStream( + new LockingScheme( + new AlluxioURI("/"), InodeTree.LockPattern.READ, true), + mFileSystemMaster, mFileSystemMaster.getSyncPathCache(), + RpcContext.NOOP, DescendantType.ALL, FileSystemMasterCommonPOptions.getDefaultInstance(), + false, + false, + false); + + InodeSyncStream iss1 = inodeSyncStreamSupplier.get(); + InodeSyncStream iss2 = inodeSyncStreamSupplier.get(); + assertSyncHappenTwice(syncConcurrent(iss1, iss2)); + assertSyncHappenTwice(syncSequential(inodeSyncStreamSupplier, inodeSyncStreamSupplier)); + } + private void assertTheSecondSyncSkipped( Pair results) { assertEquals(InodeSyncStream.SyncStatus.OK, results.getFirst());