Skip to content

Commit

Permalink
Avoid getting meta sync lock if no need
Browse files Browse the repository at this point in the history
### What changes are proposed in this pull request?

Avoid getting meta sync lock if no need.

### Why are the changes needed?

After backporting Alluxio#16241, "ls /" encountered the following exception

```
2023-03-30 14:00:51,657 ERROR FileSystemMasterClientServiceHandler -
Exit (Error): ListStatus: request=path: "/"
options {
loadMetadataType: ONCE
commonOptions {
syncIntervalMs: 86400000
ttl: -1
ttlAction: DELETE
}
recursive: false
loadMetadataOnly: false
}

java.lang.RuntimeException: Call cancelled by trackers:
GRPC_CLIENT_TRACKER
at alluxio.master.file.RpcContext.throwIfCancelled(RpcContext.java:107)
at alluxio.master.file.InodeSyncStream.sync(InodeSyncStream.java:322)
at
alluxio.master.file.DefaultFileSystemMaster.syncMetadata(DefaultFileSystemMaster.java:3816)
at
alluxio.master.file.DefaultFileSystemMaster.listStatus(DefaultFileSystemMaster.java:1042)
...
```

We found that "ls /" was waiting for the WRITE lock of "/" in
MetadataSyncLockManager even it did not need to sync metadata. As other
clients were "ls" other paths and they hold READ lock on "/", "ls /" got
stuck.

### Does this PR introduce any user facing changes?

NO

pr-link: Alluxio#17172
change-id: cid-405084e218b150b4ffcce08294b30b3dd867a309
  • Loading branch information
secfree authored and jiacheliu3 committed May 19, 2023
1 parent 6c703fa commit 5b00606
Showing 1 changed file with 6 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,12 @@ public InodeSyncStream(LockingScheme rootScheme, DefaultFileSystemMaster fsMaste
* @return SyncStatus object
*/
public SyncStatus sync() throws AccessControlException, InvalidPathException {
LOG.debug("Running InodeSyncStream on path {}, with status {}, and force sync {}",
mRootScheme.getPath(), mRootScheme.shouldSync(), mForceSync);
if (!mRootScheme.shouldSync().isShouldSync() && !mForceSync) {
DefaultFileSystemMaster.Metrics.INODE_SYNC_STREAM_SKIPPED.inc();
return SyncStatus.NOT_NEEDED;
}
if (!mDedupConcurrentSync) {
return syncInternal();
}
Expand All @@ -430,12 +436,6 @@ private SyncStatus syncInternal() throws
int failedSyncPathCount = 0;
int skippedSyncPathCount = 0;
int stopNum = -1; // stop syncing when we've processed this many paths. -1 for infinite
LOG.debug("Running InodeSyncStream on path {}, with status {}, and force sync {}",
mRootScheme.getPath(), mRootScheme.shouldSync(), mForceSync);
if (!mRootScheme.shouldSync().isShouldSync() && !mForceSync) {
DefaultFileSystemMaster.Metrics.INODE_SYNC_STREAM_SKIPPED.inc();
return SyncStatus.NOT_NEEDED;
}
if (mDedupConcurrentSync && mRootScheme.shouldSync() != SyncCheck.SHOULD_SYNC) {
/*
* If a concurrent sync on the same path is successful after this sync had already
Expand Down

0 comments on commit 5b00606

Please sign in to comment.