-
Notifications
You must be signed in to change notification settings - Fork 408
[CELEBORN-2236] Avoiding regular expressions for DiskFileInfo storage type determination #3562
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
Conversation
b88b8e7 to
56215ea
Compare
7d9f544 to
8b883fe
Compare
491cee8 to
7422104
Compare
RexXiong
left a comment
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, thanks!
common/src/main/scala/org/apache/celeborn/common/util/PbSerDeUtils.scala
Outdated
Show resolved
Hide resolved
common/src/test/scala/org/apache/celeborn/common/util/PbSerDeUtilsTest.scala
Show resolved
Hide resolved
5bc3db7 to
b8fee72
Compare
16ebffd to
508cdea
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3562 +/- ##
==========================================
+ Coverage 67.05% 67.15% +0.10%
==========================================
Files 357 357
Lines 21779 21860 +81
Branches 1930 1943 +13
==========================================
+ Hits 14602 14677 +75
- Misses 6160 6164 +4
- Partials 1017 1019 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
508cdea to
1d6beba
Compare
1d6beba to
97e6be8
Compare
97e6be8 to
f4b3947
Compare
|
Thanks, merge to main(v0.7.0) |
What changes were proposed in this pull request?
Avoiding regular expressions for DiskFileInfo storage type determination.
Why are the changes needed?
When sending a heartbeat, the Worker iterates all FileInfo objects and uses regex matching on a large number of them to check if the file is an HDFS file, thus reducing processing efficiency.
Does this PR resolve a correctness bug?
No.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
CI.