Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check cluster ufs version in getClusterConf #18222

Merged
merged 1 commit into from
Oct 13, 2023
Merged

Check cluster ufs version in getClusterConf #18222

merged 1 commit into from
Oct 13, 2023

Conversation

Haoning-Sun
Copy link
Contributor

What changes are proposed in this pull request?

Check cluster ufs version in getClusterConf.

Why are the changes needed?

Fix #18221.

@jja725
Copy link
Contributor

jja725 commented Oct 2, 2023

hey @jiacheliu3 , do you mind taking a look at this change? The change looks good to me but not sure that's the correct fix.

@yyongycy
Copy link
Contributor

@jja725 what is your opinion?

yyongycy
yyongycy reviewed Oct 12, 2023
View reviewed changes
core/common/src/main/java/alluxio/conf/Configuration.java
@@ -493,6 +494,11 @@ public static InstancedConfiguration getClusterConf(GetConfigurationPResponse re
scope, clientVersion, clusterVersion);
clusterProps.remove(PropertyKey.VERSION);
}
// Check ufs version. Avoid adding it to user properties if the two versions are the same.
String clusterUfsVersion = clusterProps.get(PropertyKey.UNDERFS_VERSION).toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please clarify why the version caused problem?

@Haoning-Sun
Copy link
Contributor Author

Haoning-Sun commented Oct 12, 2023 •
edited
Loading

@yyongycy
The #18221 error is as follows

java.lang.IllegalArgumentException: No Under File System Factory found for: o3fs://xxx.xxx/
        at alluxio.underfs.UnderFileSystem$Factory.create(UnderFileSystem.java:95)
        at alluxio.underfs.AbstractUfsManager.getOrAdd(AbstractUfsManager.java:132)
        at alluxio.underfs.AbstractUfsManager.lambda$addMount$0(AbstractUfsManager.java:183)
        at alluxio.underfs.UfsManager$UfsClient.acquireUfsResource(UfsManager.java:61)
        at alluxio.worker.grpc.UfsFileWriteHandler.createUfsFile(UfsFileWriteHandler.java:155)
        at alluxio.worker.grpc.UfsFileWriteHandler.writeBuf(UfsFileWriteHandler.java:139)
        at alluxio.worker.grpc.UfsFileWriteHandler.writeBuf(UfsFileWriteHandler.java:49)
        at alluxio.worker.grpc.AbstractWriteHandler.writeData(AbstractWriteHandler.java:281)
        at alluxio.worker.grpc.AbstractWriteHandler.lambda$writeDataMessage$1(AbstractWriteHandler.java:179)
        at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:123)
        at alluxio.worker.grpc.GrpcExecutors$ImpersonateThreadPoolExecutor.lambda$execute$0(GrpcExecutors.java:159)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

The ufs info is obtained by mMasterClient.getUfsInfo. If underfs.version is set during the mount operation, the value in the configuration will be overwritten in createMountSpecificConf, otherwise the value in the current configuration will be used when creating ufs client.
image
image

yyongycy
yyongycy approved these changes Oct 13, 2023
View reviewed changes
Copy link
Contributor

@yyongycy yyongycy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yyongycy yyongycy added the type-bug This issue is about a bug label Oct 13, 2023
@yyongycy
Copy link
Contributor

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 87ddb3c into Alluxio:master-2.x Oct 13, 2023
17 checks passed
@jiacheliu3 jiacheliu3 mentioned this pull request Nov 8, 2023
Merged
alluxio-bot added a commit that referenced this pull request Nov 8, 2023
@alluxio-bot
Merge master-2.x commits 2023/07/01~2023/11/08 into main
c2f4cd2 
### What changes are proposed in this pull request?
Merge missing commits from master-2.x to main. The commits in 2023/07/01~2023/11/08 from main...master-2.x will be included by this PR.

We do this merge to catch missing fixes from `master-2.x` and catch the train before `main` cuts a release.

#17747 is not cherry picked because tencent cloud EMR doc is removed
#17755 is not cherry picked because DistLoadCliRunner has been removed in 3.x
#17758 is not cherry picked because MonoBlockStore has been removed in 3.x
#17641 is not cherry picked because the PR has already been in main
#17781 is not cherry picked because the PR has already been in main
#17722 is not cherry picked because the alluxio-fuse command has been changed a lot
#17489 is not cherry picked because audit log on master is no longer in 3.x
#17865 is not cherry picked because replication on job service is no longer in 3.x
#17858 is not cherry picked because it is already in main
#18090 is not cherry picked because generate-tarball has been rewritten in 3.x
#18091 is not cherry picked because the change is already in main
#17474 is not cherry picked because reconfiguration feature is not defined in 3.x
#17735 is not cherry picked because MonoBlockStore is no longer in 3.x
#18133 is not cherry picked because the issue is about master metadata and no longer relevant in 3.x
#17910 is not cherry picked because I prefer to do that manually
#17983 is not cherry picked because the web UI has been reworked
#17984 is not cherry picked because Mount/Unmount commands have been reworked in 3.x
#18103 is not cherry picked because worker cache metrics have been reworked in 3.x
#18185 is not cherry picked because the report command has been reworked in 3.x
#18222 is not cherry picked because Mount/Unmount operations have been reworked in 3.x
#18143 is not cherry picked because the change is already in main
#18303 is not cherry picked because the change is already in main
#18208 is not cherry picked because cache metrics have been reworked in 3.x
#17002 is not cherry picked because the owner has been notified separately
#18334 is not cherry picked because the bash scripts have been reworked in 3.x
#18326 is not cherry picked because the owner has been notified separately

			pr-link: #18397
			change-id: cid-dbf8cbb2d9e721a5a0a1e5028a3c9577438a2ac0
maobaolong pushed a commit to maobaolong/alluxio that referenced this pull request Jan 3, 2024
@Haoning-Sun @codings-dan
Check cluster ufs version in getClusterConf
2ac77f8 
### What changes are proposed in this pull request?

Check cluster ufs version in getClusterConf.

### Why are the changes needed?

Fix Alluxio#18221.

			pr-link: Alluxio#18222
			change-id: cid-2c67758e30cac31a84f41d47b50c0ed958c88ffb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yyongycy yyongycy yyongycy approved these changes

@jja725 jja725 jja725 approved these changes

@jiacheliu3 jiacheliu3 Awaiting requested review from jiacheliu3

Assignees
No one assigned
Labels
type-bug This issue is about a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Haoning-Sun @jja725 @yyongycy @alluxio-bot