Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
hadoop-yetus commented on PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#issuecomment-2172465823
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 19s | | Docker mode activated. |
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files
found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain
any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include
any new or modified tests. Please justify why no new tests are needed for this
patch. Also please list what manual steps were performed to verify this patch.
|
_ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 32m 30s | | trunk passed |
| +1 :green_heart: | compile | 0m 42s | | trunk passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | compile | 0m 38s | | trunk passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | checkstyle | 0m 39s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 39s | | trunk passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javadoc | 1m 6s | | trunk passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | spotbugs | 1m 44s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 25s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 37s | | the patch passed |
| +1 :green_heart: | compile | 0m 36s | | the patch passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javac | 0m 36s | | the patch passed |
| +1 :green_heart: | compile | 0m 33s | | the patch passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | javac | 0m 33s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 0m 30s |
[/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/8/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt)
| hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 125 unchanged
- 1 fixed = 126 total (was 126) |
| +1 :green_heart: | mvnsite | 0m 34s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 30s | | the patch passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javadoc | 1m 0s | | the patch passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | spotbugs | 1m 46s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 52s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 203m 29s | | hadoop-hdfs in the patch
passed. |
| +1 :green_heart: | asflicense | 0m 30s | | The patch does not
generate ASF License warnings. |
| | | 289m 38s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.45 ServerAPI=1.45 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/8/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/6809 |
| JIRA Issue | HDFS-17518 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux b8372779f04d 5.15.0-106-generic #116-Ubuntu SMP Wed Apr 17
09:17:56 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / d17c06b0113f874a02b335de9566a4481e9705e3 |
| Default Java | Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/8/testReport/ |
| Max. process+thread count | 4054 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U:
hadoop-hdfs-project/hadoop-hdfs |
| Console output |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/8/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
hadoop-yetus commented on PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#issuecomment-2171832002
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 18s | | Docker mode activated. |
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files
found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain
any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include
any new or modified tests. Please justify why no new tests are needed for this
patch. Also please list what manual steps were performed to verify this patch.
|
_ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 32m 23s | | trunk passed |
| +1 :green_heart: | compile | 0m 42s | | trunk passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | compile | 0m 39s | | trunk passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | checkstyle | 0m 39s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 42s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 39s | | trunk passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javadoc | 1m 6s | | trunk passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | spotbugs | 1m 45s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 2s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 34s | | the patch passed |
| +1 :green_heart: | compile | 0m 35s | | the patch passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javac | 0m 35s | | the patch passed |
| +1 :green_heart: | compile | 0m 34s | | the patch passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | javac | 0m 34s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 0m 29s |
[/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/7/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt)
| hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 125 unchanged
- 1 fixed = 126 total (was 126) |
| +1 :green_heart: | mvnsite | 0m 36s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 30s | | the patch passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javadoc | 1m 2s | | the patch passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | spotbugs | 1m 43s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 28s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 205m 36s | | hadoop-hdfs in the patch
passed. |
| +1 :green_heart: | asflicense | 0m 28s | | The patch does not
generate ASF License warnings. |
| | | 291m 45s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.45 ServerAPI=1.45 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/7/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/6809 |
| JIRA Issue | HDFS-17518 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux d1d6938c241a 5.15.0-106-generic #116-Ubuntu SMP Wed Apr 17
09:17:56 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 1fdb6d153f2c7df0d7fe17effc3119d16028d404 |
| Default Java | Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/7/testReport/ |
| Max. process+thread count | 4028 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U:
hadoop-hdfs-project/hadoop-hdfs |
| Console output |
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/7/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
hadoop-yetus commented on PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#issuecomment-215932
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 14m 2s | | Docker mode activated. |
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files
found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain
any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include
any new or modified tests. Please justify why no new tests are needed for this
patch. Also please list what manual steps were performed to verify this patch.
|
_ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 32m 20s | | trunk passed |
| +1 :green_heart: | compile | 0m 45s | | trunk passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | compile | 0m 40s | | trunk passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | checkstyle | 0m 40s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 43s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 42s | | trunk passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javadoc | 1m 7s | | trunk passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | spotbugs | 1m 46s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 10s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 36s | | the patch passed |
| +1 :green_heart: | compile | 0m 36s | | the patch passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javac | 0m 36s | | the patch passed |
| +1 :green_heart: | compile | 0m 34s | | the patch passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | javac | 0m 34s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| -0 :warning: | checkstyle | 0m 27s |
[/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/6/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt)
| hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 125 unchanged
- 1 fixed = 126 total (was 126) |
| +1 :green_heart: | mvnsite | 0m 40s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 29s | | the patch passed with JDK
Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javadoc | 1m 1s | | the patch passed with JDK
Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | spotbugs | 1m 42s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 1s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| -1 :x: | unit | 205m 4s |
[/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/6/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt)
| hadoop-hdfs in the patch passed. |
| +1 :green_heart: | asflicense | 0m 29s | | The patch does not
generate ASF License warnings. |
| | | 305m 55s | | |
| Reason | Tests |
|---:|:--|
| Failed junit tests | hadoop.hdfs.TestDFSStripedInputStream |
| | hadoop.hdfs.server.datanode.TestDirectoryScanner |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.45 ServerAPI=1.45 base:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/6/artifact/out/Dockerfile
|
| GITHUB PR | https://github.com/apache/hadoop/pull/6809 |
| JIRA Issue | HDFS-17518 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 266e4e61c060 5.15.0-106-generic #116-Ubuntu SMP Wed Apr 17
09:17:56 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 59fa6bd8182f61e64342445ca437c456ee12f68f |
| Default Java | Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| Multi-JDK versions |
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
/usr/lib/jvm/java-8-openjdk-amd64:Private
Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| Test Results |
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1636965918
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
I would say keep the checkLeases() return logic as is.. except the above
changes.
Because there are possibilities where `needSync = true` may not be true at
all. So calling logSync() when unnecessary can be avoided.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
ThinkerLei commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1611850764
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
@vinayakumarb Thank you for your reply. How about changing the method
`checkLeases` to return true?
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1608866841
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
as mentioned above, please change below logic to call logSync() always.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1608864957
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
I dont think that special case needs to be handled. If there is no txn, then
also calling logSync() wont be a problem.
If there is no edit txn, logSync() will just return without doing anything.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
ThinkerLei commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1603594294
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
@vinayakumarb Thank you for your reply. I understand what you mean. But
there is another case where calling `logSync()` is not required as mentioned in
[HDFS-17519](https://issues.apache.org/jira/browse/HDFS-17519.) Do we need to
consider this scenario described in
[HDFS-17519](https://issues.apache.org/jira/browse/HDFS-17519)?
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r160870
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
@Hexiaoqiao Its perfectly fine if concurrently other write operations also
call`logSync()`. This transaction also will get included in the logSync()
called by other threads. Its true for vice versa as well.
@ThinkerLei I wanted to make the if-else condition simple.
```
boolean completed=fsn.internalReleaseLease();
if (!needSync)) {
needSync = true;
}
```
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
ThinkerLei commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1600246934
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
@Hexiaoqiao @vinayakumarb Thank you very much for your comment. In the one
hand, we may indeed not need to invoke logSync() in time. The purpose of this
modification is to ensure that `editlog` can be `sync` in a timely manner like
other write operations，so as to prevent the loss of the `editlog` in some
extreme cases. on the other hand, @vinayakumarb I'm still a little confused by
what you're saying. The current modification
```
boolean isClosed = !lastINode.isUnderConstruction();
if (!needSync && (!completed || isClosed)) {
needSync = true;
}
```
has ensured that leaseMonitor can invoke `logSync()` when the file gets
closed and `reassign lease`. File gets closed, `isClosed` will be true. Lease
reassigned , `completed` will be false and the initial value of `needSync` is
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
Hexiaoqiao commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1599314271
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
Thanks, I totally agree to write and sync edit log here where 'write'
already do that for both `close` and `reassign lease` but miss `sync` for some
corner case. My point is do we need to sync them invoke `logSync()` in time?
And what will it happen if sync by other write operation at following because
edit from LeaseManager.Monitor is one asynchronous logic which is not have to
in order IMO. Maybe it could be going to be missed if there are no other write
operation later then NameNode shutdown?
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1599005240
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
On the other hand, if we see the complete logic of `internalReleaseLease()`
we need to call logSync() always.
There are two possibilities overall.
1. File gets closed.
2. lease recovery gets initiated with reassign of lease.
In both of above cases, there will be edit txn logged. So need to call
logSync().
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
Hexiaoqiao commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1598536353
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
> I would recommend you to change the return type of internalReleaseLease()
to ImmutablePair to include both completed and needSync
values.
needSync will be true in both cases of file closed and lease re-assignment.
+1. If we will plan to improve it, should fix it together.
BTW, what will it happen if not sync in time, LeaseManager.Monitor is one
asynchronous logic, it can not be ensure to sync edits in one certain order
right?
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
ThinkerLei commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1597807172
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
@vinayakumarb Thank you for your review. Let me explain the current logic.
The logic I am modifying now is as follows: if the lease is recovered or the
lease is reassigned, it will return false, just like the previous logic. Then,
in the checkLeases method, if the return is false and needSync is false,
needSync will be reset to true. Subsequently, the edits log will be flushed by
leaseMonitor. This way, when RPCs such as recoverLease call the
internalReleaseLease method, they can remain consistent with the original
behavior.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
ThinkerLei commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1597806803
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -3738,7 +3738,7 @@ boolean internalReleaseLease(Lease lease, String src,
INodesInPath iip,
NameNode.stateChangeLog.warn("BLOCK*" +
" internalReleaseLease: All existing blocks are COMPLETE," +
" lease removed, file " + src + " closed.");
- return true; // closed!
+ return false; // closed!
Review Comment:
@vinayakumarb Thank you for your review. Let me explain the current logic.
The logic I am modifying now is as follows: if the lease is recovered or the
lease is reassigned, it will return false, just like the previous logic. Then,
in the checkLeases method, if the return is false and needSync is false,
needSync will be reset to true. Subsequently, the edits log will be flushed by
leaseMonitor. This way, when RPCs such as recoverLease call the
internalReleaseLease method, they can remain consistent with the original
behavior.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
ThinkerLei commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1597806803
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -3738,7 +3738,7 @@ boolean internalReleaseLease(Lease lease, String src,
INodesInPath iip,
NameNode.stateChangeLog.warn("BLOCK*" +
" internalReleaseLease: All existing blocks are COMPLETE," +
" lease removed, file " + src + " closed.");
- return true; // closed!
+ return false; // closed!
Review Comment:
@vinayakumarb Thank you for your review. Let me explain the current logic.
The logic I am modifying now is as follows: if the lease is recovered or the
lease is reassigned, it will return false, just like the previous logic. Then,
in the checkLeases method, if the return is false and needSync is false,
needSync will be reset to true. Subsequently, the edits log will be flushed by
leaseMonitor. This way, when RPCs such as recoverLease call the
internalReleaseLease method, they can remain consistent with the original
behavior.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1597693783
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java:
##
@@ -626,7 +626,8 @@ private synchronized boolean checkLeases(Collection
leasesToCheck) {
}
}
// If a lease recovery happened, we need to sync later.
Review Comment:
This is a nice hack. But this will not handle the case, where actual
recovery of the file is triggered and lease is reassigned.
Lease re-assignment also will have a edit log. This also should be synced as
well.
I would recommend you to change the return type of `internalReleaseLease()`
to `ImmutablePair` to include both `completed` and `needSync`
values.
needSync will be true in both cases of file closed and lease re-assignment.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
hadoop-yetus commented on PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#issuecomment-2106325014
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 00s | | No case conflicting files
found. |
| +0 :ok: | spotbugs | 0m 01s | | spotbugs executables are not
available. |
| +0 :ok: | codespell | 0m 01s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 01s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 00s | | The patch does not contain
any @author tags. |
| -1 :x: | test4tests | 0m 00s | | The patch doesn't appear to include
any new or modified tests. Please justify why no new tests are needed for this
patch. Also please list what manual steps were performed to verify this patch.
|
_ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 110m 34s | | trunk passed |
| +1 :green_heart: | compile | 7m 49s | | trunk passed |
| +1 :green_heart: | checkstyle | 6m 17s | | trunk passed |
| +1 :green_heart: | mvnsite | 8m 58s | | trunk passed |
| +1 :green_heart: | javadoc | 8m 27s | | trunk passed |
| +1 :green_heart: | shadedclient | 181m 41s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 01s | | the patch passed |
| +1 :green_heart: | compile | 4m 44s | | the patch passed |
| +1 :green_heart: | javac | 4m 44s | | the patch passed |
| +1 :green_heart: | blanks | 0m 00s | | The patch has no blanks
issues. |
| +1 :green_heart: | checkstyle | 3m 22s | | the patch passed |
| +1 :green_heart: | mvnsite | 5m 12s | | the patch passed |
| +1 :green_heart: | javadoc | 4m 25s | | the patch passed |
| +1 :green_heart: | shadedclient | 195m 47s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | asflicense | 7m 19s | | The patch does not
generate ASF License warnings. |
| | | 523m 52s | | |
| Subsystem | Report/Notes |
|--:|:-|
| GITHUB PR | https://github.com/apache/hadoop/pull/6809 |
| JIRA Issue | HDFS-17518 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | MINGW64_NT-10.0-17763 117457eea4e9 3.4.10-87d57229.x86_64
2024-02-14 20:17 UTC x86_64 Msys |
| Build tool | maven |
| Personality | /c/hadoop/dev-support/bin/hadoop.sh |
| git revision | trunk / 6c1e9a137d17034a7f2b2a36b3b9ce0f6afb5ac9 |
| Default Java | Azul Systems, Inc.-1.8.0_332-b09 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6809/2/testReport/
|
| modules | C: hadoop-hdfs-project/hadoop-hdfs U:
hadoop-hdfs-project/hadoop-hdfs |
| Console output |
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6809/2/console
|
| versions | git=2.44.0.windows.1 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
hadoop-yetus commented on PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#issuecomment-2106300111
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 00s | | No case conflicting files
found. |
| +0 :ok: | spotbugs | 0m 01s | | spotbugs executables are not
available. |
| +0 :ok: | codespell | 0m 01s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 01s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 00s | | The patch does not contain
any @author tags. |
| -1 :x: | test4tests | 0m 00s | | The patch doesn't appear to include
any new or modified tests. Please justify why no new tests are needed for this
patch. Also please list what manual steps were performed to verify this patch.
|
_ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 87m 17s | | trunk passed |
| +1 :green_heart: | compile | 5m 56s | | trunk passed |
| +1 :green_heart: | checkstyle | 4m 41s | | trunk passed |
| +1 :green_heart: | mvnsite | 6m 23s | | trunk passed |
| +1 :green_heart: | javadoc | 6m 00s | | trunk passed |
| +1 :green_heart: | shadedclient | 142m 05s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 30s | | the patch passed |
| +1 :green_heart: | compile | 3m 22s | | the patch passed |
| +1 :green_heart: | javac | 3m 22s | | the patch passed |
| +1 :green_heart: | blanks | 0m 01s | | The patch has no blanks
issues. |
| +1 :green_heart: | checkstyle | 2m 18s | | the patch passed |
| +1 :green_heart: | mvnsite | 4m 01s | | the patch passed |
| +1 :green_heart: | javadoc | 3m 28s | | the patch passed |
| +1 :green_heart: | shadedclient | 155m 05s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | asflicense | 5m 13s | | The patch does not
generate ASF License warnings. |
| | | 411m 17s | | |
| Subsystem | Report/Notes |
|--:|:-|
| GITHUB PR | https://github.com/apache/hadoop/pull/6809 |
| JIRA Issue | HDFS-17518 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | MINGW64_NT-10.0-17763 b1f57066a6fa 3.4.10-87d57229.x86_64
2024-02-14 20:17 UTC x86_64 Msys |
| Build tool | maven |
| Personality | /c/hadoop/dev-support/bin/hadoop.sh |
| git revision | trunk / 6c1e9a137d17034a7f2b2a36b3b9ce0f6afb5ac9 |
| Default Java | Azul Systems, Inc.-1.8.0_332-b09 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6809/3/testReport/
|
| modules | C: hadoop-hdfs-project/hadoop-hdfs U:
hadoop-hdfs-project/hadoop-hdfs |
| Console output |
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6809/3/console
|
| versions | git=2.44.0.windows.1 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
ThinkerLei commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1597594907
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -3738,7 +3738,7 @@ boolean internalReleaseLease(Lease lease, String src,
INodesInPath iip,
NameNode.stateChangeLog.warn("BLOCK*" +
" internalReleaseLease: All existing blocks are COMPLETE," +
" lease removed, file " + src + " closed.");
- return true; // closed!
+ return false; // closed!
Review Comment:
@vinayakumarb Thank you very much for your comment; it is very accurate. I
have modified. Please review again.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
hadoop-yetus commented on PR #6809: URL: https://github.com/apache/hadoop/pull/6809#issuecomment-2106176232 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 0s | | Docker mode activated. | | -1 :x: | patch | 0m 15s | | https://github.com/apache/hadoop/pull/6809 does not apply to trunk. Rebase required? Wrong Branch? See https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help. | | Subsystem | Report/Notes | |--:|:-| | GITHUB PR | https://github.com/apache/hadoop/pull/6809 | | JIRA Issue | HDFS-17518 | | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6809/3/console | | versions | git=2.34.1 | | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org | This message was automatically generated. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1597551434
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -3738,7 +3738,7 @@ boolean internalReleaseLease(Lease lease, String src,
INodesInPath iip,
NameNode.stateChangeLog.warn("BLOCK*" +
" internalReleaseLease: All existing blocks are COMPLETE," +
" lease removed, file " + src + " closed.");
- return true; // closed!
+ return false; // closed!
Review Comment:
As per javadoc of this method, return value indicates whether file was
closed or not.
Changing that value here, may solve the problem of logSync() particularly in
this case, but it will be problematic for other usages of this method.
For ex: recoverLease() RPC will get false, even though file was closed.
As per the javadoc, even if the return value is false, there are edits
logged (reassigning the lease, when blockrecovery is initiated.).
So calling the logSync() is required in both these cases.
That said, Cannot blindly call logSync() always.
So, more correct approach to fix this is to return a combination of these
values from this method (i.e. complerted and needsync )
And determine whether to call sync or not in the caller.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
vinayakumarb commented on code in PR #6809:
URL: https://github.com/apache/hadoop/pull/6809#discussion_r1597551434
##
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##
@@ -3738,7 +3738,7 @@ boolean internalReleaseLease(Lease lease, String src,
INodesInPath iip,
NameNode.stateChangeLog.warn("BLOCK*" +
" internalReleaseLease: All existing blocks are COMPLETE," +
" lease removed, file " + src + " closed.");
- return true; // closed!
+ return false; // closed!
Review Comment:
As per javadoc of this method, return value indicates whether file was
closed or not.
Changing that value here, may solve the problem of logSync() particularly in
this case, but it will be problematic for other usages of this method.
For ex: recoverLease() RPC will always get false, even though file was
closed.
As per the javadoc, even if the return value is false, there are edits
logged (reassigning the lease, when blockrecovery is initiated.).
So calling the logSync() is required in both these cases.
That said, Cannot blindly call logSync() always.
So, more correct approach to fix this is to return a combination of these
values from this method (i.e. complerted and needsync )
And determine whether to call sync or not in the caller.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[PR] HDFS-17518: Sync the editslog if a file is closed in the lease monitor [hadoop]
ThinkerLei opened a new pull request, #6809: URL: https://github.com/apache/hadoop/pull/6809 In the lease monitor, if a file is closed, method checklease will return true, and then the edits log will not be sync. In my opinion, we should sync the edits log to avoid not synchronizing the state to the standby NameNode for a long time. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
