Skip to content

[CELEBORN-1414] PartitionFilesSorter resolve DiskFileInfo without sorting lock#2498

Closed
SteNicholas wants to merge 1 commit intoapache:mainfrom
SteNicholas:CELEBORN-1414
Closed

[CELEBORN-1414] PartitionFilesSorter resolve DiskFileInfo without sorting lock#2498
SteNicholas wants to merge 1 commit intoapache:mainfrom
SteNicholas:CELEBORN-1414

Conversation

@SteNicholas
Copy link
Member

@SteNicholas SteNicholas commented May 9, 2024

What changes were proposed in this pull request?

PartitionFilesSorter calls resolve of DiskFileInfo without sorting lock to reduce the lock scope for performance improvement of getSortedFileInfo.

Why are the changes needed?

PartitionFilesSorter#resolve is thread safe. Therefore, PartitionFilesSorter invokes resolve with sorting lock at present, which does not need to lock the resolving of DiskFileInfo. PartitionFilesSorter could resolve DiskFileInfo without sorting lock to improve performance of getSortedFileInfo.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

GA.

@SteNicholas SteNicholas marked this pull request as draft May 9, 2024 11:20
@SteNicholas SteNicholas force-pushed the CELEBORN-1414 branch 2 times, most recently from 83ab37e to e4566d1 Compare May 9, 2024 13:58
@SteNicholas SteNicholas marked this pull request as ready for review May 9, 2024 18:11
@SteNicholas
Copy link
Member Author

SteNicholas commented May 9, 2024

Ping @RexXiong, @FMX, @cfmcgrady.

@SteNicholas SteNicholas requested review from FMX and cfmcgrady May 10, 2024 03:25
Copy link
Contributor

@RexXiong RexXiong left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@SteNicholas
Copy link
Member Author

@FMX, @cfmcgrady, PTAL.

Copy link
Contributor

@AngersZhuuuu AngersZhuuuu left a comment

Choose a reason for hiding this comment

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

LGTM

@SteNicholas
Copy link
Member Author

Merging to main(v0.5.0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants