-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
Support integrating TOS as the UnderFileSystem #18621
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good, left some comments to fix, thanks for your contribution!
.build(); | ||
public static final PropertyKey TOS_REGION = | ||
stringBuilder(Name.TOS_REGION) | ||
.setDescription("The region name of TOS bucket.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please align to the previous line.
underfs/tos/pom.xml
Outdated
<dependency> | ||
<groupId>com.volcengine</groupId> | ||
<artifactId>ve-tos-java-sdk</artifactId> | ||
<version>2.7.1</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
define the version in the root pom.xml, not here
import javax.annotation.concurrent.NotThreadSafe; | ||
|
||
/** | ||
* A stream for reading a file from COS. This input stream returns 0 when calling read with an empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* A stream for reading a file from COS. This input stream returns 0 when calling read with an empty | |
* A stream for reading a file from TOS. This input stream returns 0 when calling read with an empty |
import javax.annotation.concurrent.NotThreadSafe; | ||
|
||
/** | ||
* A stream for writing a file into COS. The data will be persisted to a temporary directory on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* A stream for writing a file into COS. The data will be persisted to a temporary directory on the | |
* A stream for writing a file into TOS. The data will be persisted to a temporary directory on the |
return new ObjectStatus(key, output.getEtag(), output.getContentLength(), | ||
lastModifiedTime); | ||
} catch (TosException e) { | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check the exception, if it's 404 not found, then return null, otherwise throw a exception
} | ||
} | ||
|
||
String err = "TOS Credentials not available, cannot create TOS Under File System."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String err = "TOS Credentials not available, cannot create TOS Under File System."; | |
String err = "TOS Credentials or configurations not available, cannot create TOS Under File System."; |
Automated checks report:
Some checks failed. Please fix the reported issues and reply |
Automated checks report:
All checks passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just remove the OSS code change
@@ -47,7 +48,8 @@ public UnderFileSystem create(String path, UnderFileSystemConfiguration conf) { | |||
} | |||
} | |||
|
|||
String err = "OSS Credentials not available, cannot create OSS Under File System."; | |||
String err = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thu-david this is the OSS UnderFileSystem Factory, please remove this line
alluxio-bot, merge this please. |
What changes are proposed in this pull request?
Created an interface for TOS, enabling Alluxio to operate and cache data specifically for TOS.
Why are the changes needed?
Tested the creation, deletion, metadata retrieval, and directory information retrieval for files of sizes 1MB, 64MB, 1GB, and 10GB. This can support the most basic TOS query requests.
Does this PR introduce any user facing changes?
This implementation continues to follow Alluxio's API. To use it, you only need to provide the TOS information in the configuration file.