-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[core] Add hint in local lookup file tracking with remote file #3814
Conversation
} | ||
builder.append("-"); | ||
} | ||
String prefix = String.format("%s-%s", builder, 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.
I am concerned that it may be too long. Can we use its hash directly as a prefix?
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.
We can know files are come from the same partition.
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.
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.
Can we cut it off? For example, if it exceeds 20 characters, do not add any more
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.
Sure, it's safer
@@ -356,13 +361,26 @@ private <T> LookupLevels<T> createLookupLevels( | |||
cacheManager, | |||
new RowCompactedSerializer(keyType).createSliceComparator()); | |||
Options options = this.options.toConfiguration(); | |||
InternalRow.FieldGetter[] getters = partitionType.fieldGetters(); | |||
StringBuilder builder = new StringBuilder(); | |||
for (InternalRow.FieldGetter getter : getters) { |
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.
Extract a common method, maybe a static method in InternalRowPartitionComputer
.
10, | ||
"data-ccbb95e7-8b8c-4549-8ca9-f553843d67ad-3.orc", | ||
100)) | ||
.isEqualTo("2024073105-10-data-ccbb95e7-8b8c-4549-8ca9-f553843d67ad-3.orc"); |
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.
I think it is better to add suffix, I mistakenly thought it was an ORC file
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 final name will include the IOManager's file suffix .channel
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.
+1
Purpose
Add the hint in local file name to track the remote the file for debugging purpose.
Tests
API and Format
Documentation