Create inode before updating the MountTable

[bobbai00]

What changes are proposed in this pull request?

1. Add method validateMount in MountTable
2. When doing mount in DefaultFileSystemMaster, load the directory metadata first, then modify the MountTable

Why are the changes needed?

One major challenge we face in #15578 is that when the mount point is added to MountTable, the corresponding inodes don't exist yet. That makes our proposed change in #15578 hard because by using a Trie<Inode> data structure the inode must exist first.

In this change we propose creating the inode before updating the MountTable, so when we update the MountTable, we can just update the MountTableTrie with the correct Inode.

Does this PR introduce any user facing changes?

Nope

[alluxio-bot]

Automated checks report:

• Commits associated with Github account: PASS
• PR title follows the conventions: FAIL
  • The title of the PR does not pass all the checks. Please fix the following issues:
    • Title is too long (103 characters). Must be at most 72 characters.

Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks.

[bobbai00]

@jiacheliu3 Liu Sang, help me review it when you have time

[alluxio-bot]

Automated checks report:

• Commits associated with Github account: PASS
• PR title follows the conventions: PASS

All checks passed!

[jiacheliu3]

@bobbai00 oops failed tests are relevant

Error: 0.188 [ERROR] Errors: 
Error: 0.188 [ERROR] alluxio.master.file.meta.AsyncUfsAbsentPathCacheTest.isAbsent
Error: 0.188 [ERROR]   Run 1: AsyncUfsAbsentPathCacheTest.before:78 » Runtime No UFS information for /mnt fo...
Error: 0.191 [ERROR]   Run 2: AsyncUfsAbsentPathCacheTest.before:78 » Runtime No UFS information for /mnt fo...

[bobbai00]

@bobbai00 oops failed tests are relevant

Error: 0.188 [ERROR] Errors: 
Error: 0.188 [ERROR] alluxio.master.file.meta.AsyncUfsAbsentPathCacheTest.isAbsent
Error: 0.188 [ERROR]   Run 1: AsyncUfsAbsentPathCacheTest.before:78 » Runtime No UFS information for /mnt fo...
Error: 0.191 [ERROR]   Run 2: AsyncUfsAbsentPathCacheTest.before:78 » Runtime No UFS information for /mnt fo...

fixed it :)

[jiacheliu3]

@bobbai00 Good to go when all the unaddressed comments are addressed. Cheers :)

[bobbai00]

Mostly LGTM, thanks for the work! Can you also double check the code and see if any more unit tests can be added?

Sure. This batch of comments is resolved now. Sorry for the delay!

[bobbai00]

One last check on Bowen's comments above. After that I will go ahead and merge this :)

What checks should I do right now?🤔

[jiacheliu3]

One last check on Bowen's comments above. After that I will go ahead and merge this :)

What checks should I do right now?🤔

https://github.com/Alluxio/alluxio/pull/15928/files#r955062042 https://github.com/Alluxio/alluxio/pull/15928/files#r932908624

[bobbai00]

One last check on Bowen's comments above. After that I will go ahead and merge this :)

What checks should I do right now?🤔

https://github.com/Alluxio/alluxio/pull/15928/files#r955062042 https://github.com/Alluxio/alluxio/pull/15928/files#r932908624

My bad. Will fix it tomorrow morning.

[jiacheliu3]

@bobbai00 Pls fix impacted test like this one. Should be straightforward. Thanks :)

Error: 5.158 [ERROR]   Run 2: FileSystemMasterTest.mountShadowDir[1] 
Expected: an instance of java.io.IOException
     but: <alluxio.exception.InvalidPathException: Mount path /hello/shadow shadows an existing path /tmp/junit5704132311085920634/ufs/hello/shadow in the parent underlying filesystem> is a alluxio.exception.InvalidPathException
Stacktrace was: alluxio.exception.InvalidPathException: Mount path /hello/shadow shadows an existing path /tmp/junit5704132311085920634/ufs/hello/shadow in the parent underlying filesystem
	at alluxio.master.file.meta.MountTable.validateMountPoint(MountTable.java:203)
	at alluxio.master.file.DefaultFileSystemMaster.mountInternal(DefaultFileSystemMaster.java:3353)
	at alluxio.master.file.DefaultFileSystemMaster.mount(DefaultFileSystemMaster.java:3327)
	at alluxio.master.file.FileSystemMasterTest.mountShadowDir(FileSystemMasterTest.java:1301)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)

[jiacheliu3]

alluxio-bot, merge this please

[bobbai00]

Thanks for the review! There are still some test failures. I will check that out later.

Jiacheng Liu @.***>于2022年8月30日 周二18:17写道：

@.**** approved this pull request.

LGTM, thanks!

— Reply to this email directly, view it on GitHub https://github.com/Alluxio/alluxio/pull/15928#pullrequestreview-1091139516, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKKWDEHB2IQRSGCLCYBFKZLV32XCJANCNFSM54GR4SUQ . You are receiving this because you were mentioned.Message ID: @.***>