Skip to content
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

Try to use jdk api to create hardlink when rename file when compaction. #3876

Merged
merged 13 commits into from
Jan 8, 2024

Conversation

lifepuzzlefun
Copy link
Contributor

@lifepuzzlefun lifepuzzlefun commented Mar 18, 2023

Motivation

Current HardLink will create a process to execute mv like command to rename file in compaction logic.

maybe we can just use jdk api to do this with lower overhead.

see javadoc: https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#createLink(java.nio.file.Path,%20java.nio.file.Path)

Changes

test if Files.createLink is available in hardlink static code block.
if test fails means Files.createLink is not available.

else will use Files.createLink when call createHardlink

@lifepuzzlefun lifepuzzlefun changed the title Try to use atomic move when rename file when compaction. Try to use jdk api to create hardlink when rename file when compaction. Mar 18, 2023
@lifepuzzlefun
Copy link
Contributor Author

@hangc0276 @eolivelli @StevenLuMT can you take a look ?

@shoothzj
Copy link
Member

IIUC, hardlink will result in two files with same inode, which is different from mv command?

@lifepuzzlefun
Copy link
Contributor Author

IIUC, hardlink will result in two files with same inode, which is different from mv command?

thanks for review. the current code will create a process
to do the whole thing which is much slower than in-process api call.

Copy link
Member

@shoothzj shoothzj left a comment

Choose a reason for hiding this comment

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

LGTM

@hangc0276 hangc0276 closed this Apr 23, 2023
@hangc0276 hangc0276 reopened this Apr 23, 2023
@hangc0276
Copy link
Contributor

@lifepuzzlefun Thanks for your contribution. Would you please add a test to cover this change?

@lifepuzzlefun
Copy link
Contributor Author

@lifepuzzlefun Thanks for your contribution. Would you please add a test to cover this change?

sorry for late reply. a unit test to cover this change is uploaded. :-)

@zymap
Copy link
Member

zymap commented Aug 29, 2023

@lifepuzzlefun Could you please try rebase master to retrigger the ci?

@lifepuzzlefun
Copy link
Contributor Author

@lifepuzzlefun Could you please try rebase master to retrigger the ci?

rebased. : -)

@zymap
Copy link
Member

zymap commented Dec 4, 2023

@lifepuzzlefun Would you like to fix the style error

@hangc0276
Copy link
Contributor

@lifepuzzlefun Would you please fix the license issue?

@hangc0276
Copy link
Contributor

Move to the next release first. If this license can be fixed before the 4.16.4 release is triggered, I will include this PR.

@hangc0276 hangc0276 merged commit af419cc into apache:master Jan 8, 2024
16 checks passed
hangc0276 pushed a commit to hangc0276/bookkeeper that referenced this pull request Jan 18, 2024
…n. (apache#3876)

### Motivation

Current HardLink will create a process to execute mv like command to rename file in compaction logic.

maybe we can just use jdk api to do this with lower overhead.

see javadoc: https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#createLink(java.nio.file.Path,%20java.nio.file.Path)

### Changes

test if `Files.createLink` is available in hardlink static code block.
if test fails means `Files.createLink` is not available.

else will use `Files.createLink` when call createHardlink

(cherry picked from commit af419cc)
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
…n. (apache#3876)

### Motivation

Current HardLink will create a process to execute mv like command to rename file in compaction logic.

maybe we can just use jdk api to do this with lower overhead.

see javadoc: https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#createLink(java.nio.file.Path,%20java.nio.file.Path)

### Changes

test if `Files.createLink` is available in hardlink static code block.
if test fails means `Files.createLink` is not available.

else will use `Files.createLink` when call createHardlink
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants