You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by "lifepuzzlefun (via GitHub)" <gi...@apache.org> on 2023/03/18 11:49:02 UTC
[GitHub] [bookkeeper] lifepuzzlefun opened a new pull request, #3876: Try to use atomic move when rename file when compaction.
lifepuzzlefun opened a new pull request, #3876:
URL: https://github.com/apache/bookkeeper/pull/3876
Descriptions of the changes in this PR:
### Motivation
(Explain: why you're making that change, what is the problem you're trying to solve)
### Changes
(Describe: what changes you have made)
Master Issue: #<master-issue-number>
> ---
> In order to uphold a high standard for quality for code contributions, Apache BookKeeper runs various precommit
> checks for pull requests. A pull request can only be merged when it passes precommit checks.
>
> ---
> Be sure to do all of the following to help us incorporate your contribution
> quickly and easily:
>
> If this PR is a BookKeeper Proposal (BP):
>
> - [ ] Make sure the PR title is formatted like:
> `<BP-#>: Description of bookkeeper proposal`
> `e.g. BP-1: 64 bits ledger is support`
> - [ ] Attach the master issue link in the description of this PR.
> - [ ] Attach the google doc link if the BP is written in Google Doc.
>
> Otherwise:
>
> - [ ] Make sure the PR title is formatted like:
> `<Issue #>: Description of pull request`
> `e.g. Issue 123: Description ...`
> - [ ] Make sure tests pass via `mvn clean apache-rat:check install spotbugs:check`.
> - [ ] Replace `<Issue #>` in the title with the actual Issue number.
>
> ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 closed pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 closed pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
URL: https://github.com/apache/bookkeeper/pull/3876
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Try to use jdk api to create hardlink when rename file when compaction. [bookkeeper]
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 closed pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
URL: https://github.com/apache/bookkeeper/pull/3876
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Try to use jdk api to create hardlink when rename file when compaction. [bookkeeper]
Posted by "zymap (via GitHub)" <gi...@apache.org>.
zymap commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1837772741
@lifepuzzlefun Would you like to fix the the style error
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lifepuzzlefun commented on pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1475972868
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj commented on pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "Shoothzj (via GitHub)" <gi...@apache.org>.
Shoothzj commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1475947966
IIUC, hardlink will result in two files with same inode, which is different from *mv* command?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Try to use jdk api to create hardlink when rename file when compaction. [bookkeeper]
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 merged PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] horizonzy commented on a diff in pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "horizonzy (via GitHub)" <gi...@apache.org>.
horizonzy commented on code in PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#discussion_r1145647939
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/util/HardLink.java:
##########
@@ -416,6 +471,19 @@ public static void createHardLink(File file, File linkName)
throw new IOException(
"invalid arguments to createHardLink: link name is null");
}
+
+ // if createLink available try first, else fall back to shell command.
+ if (CREATE_LINK_SUPPORTED.get()) {
Review Comment:
We can make the `CREATE_LINK_SUPPORTED` default value true, and remove the test code.
If using createLink failed, change `CREATE_LINK_SUPPORTED` to false.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 closed pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 closed pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
URL: https://github.com/apache/bookkeeper/pull/3876
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lifepuzzlefun commented on pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1636707904
> @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. :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lifepuzzlefun commented on pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1703772924
> @lifepuzzlefun Could you please try rebase master to retrigger the ci?
rebased. : -)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lifepuzzlefun commented on pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1475931079
@hangc0276 @eolivelli @StevenLuMT can you take a look ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] horizonzy commented on a diff in pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "horizonzy (via GitHub)" <gi...@apache.org>.
horizonzy commented on code in PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#discussion_r1145647939
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/util/HardLink.java:
##########
@@ -416,6 +471,19 @@ public static void createHardLink(File file, File linkName)
throw new IOException(
"invalid arguments to createHardLink: link name is null");
}
+
+ // if createLink available try first, else fall back to shell command.
+ if (CREATE_LINK_SUPPORTED.get()) {
Review Comment:
We can make the `CREATE_LINK_SUPPORTED` default value true, and remove the test code.
If use createLink failed, change `CREATE_LINK_SUPPORTED` to false.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lifepuzzlefun commented on a diff in pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on code in PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#discussion_r1145720608
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/util/HardLink.java:
##########
@@ -416,6 +471,19 @@ public static void createHardLink(File file, File linkName)
throw new IOException(
"invalid arguments to createHardLink: link name is null");
}
+
+ // if createLink available try first, else fall back to shell command.
+ if (CREATE_LINK_SUPPORTED.get()) {
Review Comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1606025724
@lifepuzzlefun Thanks for your contribution. Would you please add a test to cover this change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Try to use jdk api to create hardlink when rename file when compaction. [bookkeeper]
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1880356793
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Try to use jdk api to create hardlink when rename file when compaction. [bookkeeper]
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1880353174
@lifepuzzlefun Would you please fix the license issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] zymap commented on pull request #3876: Try to use jdk api to create hardlink when rename file when compaction.
Posted by "zymap (via GitHub)" <gi...@apache.org>.
zymap commented on PR #3876:
URL: https://github.com/apache/bookkeeper/pull/3876#issuecomment-1696876897
@lifepuzzlefun Could you please try rebase master to retrigger the ci?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org