-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[core] Enable global index external path #6994
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
base: master
Are you sure you want to change the base?
Conversation
9e65491 to
d57cb9a
Compare
| private final long fileSize; | ||
| private final byte[] metadata; | ||
| @Nullable private final String externalPath; | ||
|
|
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.
The GlobalIndexIOMeta is intended to be a meta interface for plugin authors. However, including an external path in it might be confusing for users, as it adds complexity and is not intuitive in this context.
Would it make more sense to store only the file_path in GlobalIndexIOMeta, while leaving the external path to be handled internally by the Paimon framework? This way, the external path could be extracted and managed within the IndexFileMeta, simplifying the API for plugin authors and ensuring a clearer separation of concerns.
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.
Accepted
|
|
||
| public Path globalIndexRootDir() { | ||
| return globalIndexExternalRootDir != null ? globalIndexExternalRootDir : indexPath(); | ||
| } |
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.
If globalIndexExternalRootDir is not null, the index path be external_index_path/index/ or external_index_path/? The first one maybe better? Not sure whether retaining the index/ is necessary.
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.
Accepted, it could be like this
6a61e59 to
9330ef8
Compare
|
+1 |
Purpose
Enable global index store in external path
Tests
API and Format
Documentation