Details
Description
HDFS-14997 moved the command processing into async.
Right now, we are checking the time to add to a queue.
We should remove this one and maybe move the timing within the thread.
Attachments
Attachments
- HDFS-15075.009.patch
- 9 kB
- Xiaoqiao He
- HDFS-15075.008.patch
- 9 kB
- Xiaoqiao He
- HDFS-15075.007.patch
- 24 kB
- Xiaoqiao He
- HDFS-15075.006.patch
- 22 kB
- Xiaoqiao He
- HDFS-15075.005.patch
- 18 kB
- Xiaoqiao He
- HDFS-15075.004.patch
- 18 kB
- Xiaoqiao He
- HDFS-15075.003.patch
- 18 kB
- Xiaoqiao He
- HDFS-15075.002.patch
- 17 kB
- Xiaoqiao He
- HDFS-15075.001.patch
- 2 kB
- Xiaoqiao He
Issue Links
- is related to
-
HDFS-14997 BPServiceActor processes commands from NameNode asynchronously
- Resolved
-
HDFS-15242 Add metrics for operations hold lock times of FsDatasetImpl
- Resolved
Activity
Thanks elgoiri for your report, I would like to work on that. After move processing command to async, the timing is not the expected. The right way is time it inner thread.
v001 keep the original semantic and move tracing time to CommandProcessingThread inner. Please help to reviews. Thanks. elgoiri
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 43s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | 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 | mvninstall | 19m 28s | trunk passed |
+1 | compile | 1m 12s | trunk passed |
+1 | checkstyle | 0m 45s | trunk passed |
+1 | mvnsite | 1m 18s | trunk passed |
+1 | shadedclient | 15m 20s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 16s | trunk passed |
+1 | javadoc | 1m 17s | trunk passed |
Patch Compile Tests | |||
+1 | mvninstall | 1m 2s | the patch passed |
+1 | compile | 0m 56s | the patch passed |
+1 | javac | 0m 56s | the patch passed |
+1 | checkstyle | 0m 40s | the patch passed |
+1 | mvnsite | 1m 11s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | shadedclient | 14m 32s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 31s | the patch passed |
+1 | javadoc | 1m 18s | the patch passed |
Other Tests | |||
-1 | unit | 114m 43s | hadoop-hdfs in the patch failed. |
+1 | asflicense | 0m 33s | The patch does not generate ASF License warnings. |
179m 26s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.hdfs.TestFileChecksum |
hadoop.hdfs.TestDeadNodeDetection | |
hadoop.hdfs.server.balancer.TestBalancerRPCDelay | |
hadoop.hdfs.server.namenode.TestRedudantBlocks | |
hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots | |
hadoop.hdfs.server.aliasmap.TestSecureAliasMap | |
hadoop.hdfs.TestRollingUpgrade |
Subsystem | Report/Notes |
---|---|
Docker | Client=19.03.5 Server=19.03.5 Image:yetus/hadoop:e573ea49085 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12989256/HDFS-15075.001.patch |
Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
uname | Linux 7cef72975de5 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | trunk / f777cd3 |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_232 |
findbugs | v3.1.0-RC1 |
unit | https://builds.apache.org/job/PreCommit-HDFS-Build/28549/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/28549/testReport/ |
Max. process+thread count | 2988 (vs. ulimit of 5500) |
modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/28549/console |
Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
I would take this opportunity to improve this in two ways:
- Improve the logging logic
- Instead of having 2000 directly, make it a constant. Ideally it would be a config key, but I'm ok with just a constant.
- Use the logger format ({}) in the log message.
- Expose these times as a counter. Either using a quantile metric or something similar. Exposing this through JMX might be valuable if you had experience issues with this in the past.
Thanks elgoiri for your reviews.
v002 try to fix comments above,
a. add a config key to instead of constant 2000ms,
b. add metrics for some methods who have io operation in #datasetLock of #FsDatasetImpl.
c. improve command process monitor metric to mutableRate.
Please take another review if have times. Thanks.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 42s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | 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 | mvninstall | 21m 31s | trunk passed |
+1 | compile | 1m 0s | trunk passed |
+1 | checkstyle | 0m 48s | trunk passed |
+1 | mvnsite | 1m 15s | trunk passed |
+1 | shadedclient | 14m 57s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 18s | trunk passed |
+1 | javadoc | 1m 14s | trunk passed |
Patch Compile Tests | |||
+1 | mvninstall | 1m 8s | the patch passed |
+1 | compile | 1m 2s | the patch passed |
+1 | javac | 1m 2s | the patch passed |
-0 | checkstyle | 0m 50s | hadoop-hdfs-project/hadoop-hdfs: The patch generated 11 new + 654 unchanged - 0 fixed = 665 total (was 654) |
+1 | mvnsite | 1m 9s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 1s | The patch has no ill-formed XML file. |
+1 | shadedclient | 13m 44s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 29s | the patch passed |
+1 | javadoc | 1m 14s | the patch passed |
Other Tests | |||
-1 | unit | 126m 46s | hadoop-hdfs in the patch failed. |
+1 | asflicense | 0m 49s | The patch does not generate ASF License warnings. |
192m 41s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.hdfs.server.namenode.TestNameNodeMXBean |
hadoop.hdfs.server.blockmanagement.TestBlockStatsMXBean | |
hadoop.hdfs.TestDatanodeRegistration | |
hadoop.hdfs.server.datanode.TestBPOfferService | |
hadoop.hdfs.server.namenode.TestRedudantBlocks | |
hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl |
Subsystem | Report/Notes |
---|---|
Docker | Client=19.03.5 Server=19.03.5 Image:yetus/hadoop:c44943d1fc3 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12989460/HDFS-15075.002.patch |
Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle xml |
uname | Linux 59908fb6a0c0 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 8730a7b |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_232 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDFS-Build/28580/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt |
unit | https://builds.apache.org/job/PreCommit-HDFS-Build/28580/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/28580/testReport/ |
Max. process+thread count | 2562 (vs. ulimit of 5500) |
modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/28580/console |
Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 55s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | 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 | mvninstall | 22m 24s | trunk passed |
+1 | compile | 1m 8s | trunk passed |
+1 | checkstyle | 0m 57s | trunk passed |
+1 | mvnsite | 1m 9s | trunk passed |
+1 | shadedclient | 14m 45s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 29s | trunk passed |
+1 | javadoc | 1m 23s | trunk passed |
Patch Compile Tests | |||
+1 | mvninstall | 1m 6s | the patch passed |
+1 | compile | 1m 3s | the patch passed |
+1 | javac | 1m 3s | the patch passed |
-0 | checkstyle | 0m 50s | hadoop-hdfs-project/hadoop-hdfs: The patch generated 11 new + 654 unchanged - 0 fixed = 665 total (was 654) |
+1 | mvnsite | 1m 3s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 2s | The patch has no ill-formed XML file. |
+1 | shadedclient | 13m 30s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 38s | the patch passed |
+1 | javadoc | 1m 20s | the patch passed |
Other Tests | |||
-1 | unit | 126m 29s | hadoop-hdfs in the patch failed. |
+1 | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
193m 30s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer |
hadoop.hdfs.server.datanode.TestBPOfferService | |
hadoop.hdfs.server.namenode.TestRedudantBlocks | |
hadoop.hdfs.server.namenode.TestAddOverReplicatedStripedBlocks | |
hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl |
Subsystem | Report/Notes |
---|---|
Docker | Client=19.03.5 Server=19.03.5 Image:yetus/hadoop:c44943d1fc3 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12989460/HDFS-15075.002.patch |
Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle xml |
uname | Linux 132acc08c3ad 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 8730a7b |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_232 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDFS-Build/28581/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt |
unit | https://builds.apache.org/job/PreCommit-HDFS-Build/28581/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/28581/testReport/ |
Max. process+thread count | 2693 (vs. ulimit of 5500) |
modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/28581/console |
Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 39s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | 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 | mvninstall | 20m 52s | trunk passed |
+1 | compile | 0m 59s | trunk passed |
+1 | checkstyle | 0m 50s | trunk passed |
+1 | mvnsite | 1m 7s | trunk passed |
+1 | shadedclient | 14m 51s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 19s | trunk passed |
+1 | javadoc | 1m 13s | trunk passed |
Patch Compile Tests | |||
+1 | mvninstall | 1m 2s | the patch passed |
+1 | compile | 0m 55s | the patch passed |
+1 | javac | 0m 55s | the patch passed |
-0 | checkstyle | 0m 46s | hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 654 unchanged - 0 fixed = 655 total (was 654) |
+1 | mvnsite | 1m 3s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 2s | The patch has no ill-formed XML file. |
+1 | shadedclient | 13m 58s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 25s | the patch passed |
+1 | javadoc | 1m 9s | the patch passed |
Other Tests | |||
-1 | unit | 109m 36s | hadoop-hdfs in the patch failed. |
+1 | asflicense | 0m 32s | The patch does not generate ASF License warnings. |
174m 3s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.hdfs.server.datanode.TestNNHandlesCombinedBlockReport |
hadoop.hdfs.TestFileCreation | |
hadoop.hdfs.TestDeadNodeDetection | |
hadoop.hdfs.TestDFSInotifyEventInputStreamKerberized | |
hadoop.hdfs.TestDatanodeRegistration | |
hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks | |
hadoop.hdfs.server.namenode.TestRedudantBlocks |
Subsystem | Report/Notes |
---|---|
Docker | Client=19.03.5 Server=19.03.5 Image:yetus/hadoop:c44943d1fc3 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12989627/HDFS-15075.003.patch |
Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle xml |
uname | Linux 640831d3bd63 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | trunk / ee51ead |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_232 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDFS-Build/28587/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt |
unit | https://builds.apache.org/job/PreCommit-HDFS-Build/28587/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/28587/testReport/ |
Max. process+thread count | 2764 (vs. ulimit of 5500) |
modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/28587/console |
Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
- Can we make DFS_DATANODE_PROCESS_COMMANDS_THRESHOLD_KEY be a time period instead of fixed millis?
- In createTemporary(), I would probably have two variables instead of sharing startTimeMs. For readability.
dabecker helped with an internal review and also noticed a typo in addCovertTemporaryToTbwOp(), it should be Convert.
Thanks elgoiri and dabecker for your review comments, v004 try to fix, please take another review.
BTW, I meet another issue recently, HDFS-15113 is tracking and try to fix it, if some one want to open this feature, please pending HDFS-15113.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 28s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | 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 | mvninstall | 25m 5s | trunk passed |
+1 | compile | 1m 2s | trunk passed |
+1 | checkstyle | 0m 53s | trunk passed |
+1 | mvnsite | 1m 11s | trunk passed |
+1 | shadedclient | 13m 38s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 10s | trunk passed |
+1 | javadoc | 1m 18s | trunk passed |
Patch Compile Tests | |||
+1 | mvninstall | 1m 5s | the patch passed |
+1 | compile | 0m 55s | the patch passed |
+1 | javac | 0m 55s | the patch passed |
-0 | checkstyle | 0m 47s | hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 658 unchanged - 0 fixed = 659 total (was 658) |
+1 | mvnsite | 1m 4s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 1s | The patch has no ill-formed XML file. |
+1 | shadedclient | 14m 45s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 50s | the patch passed |
+1 | javadoc | 1m 20s | the patch passed |
Other Tests | |||
-1 | unit | 104m 4s | hadoop-hdfs in the patch failed. |
+1 | asflicense | 0m 39s | The patch does not generate ASF License warnings. |
173m 1s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks |
hadoop.hdfs.server.balancer.TestBalancerRPCDelay | |
hadoop.hdfs.server.namenode.TestRedudantBlocks | |
hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA | |
hadoop.hdfs.TestDeadNodeDetection | |
hadoop.hdfs.TestReadStripedFileWithDNFailure | |
hadoop.hdfs.TestErasureCodingMultipleRacks |
Subsystem | Report/Notes |
---|---|
Docker | Client=19.03.5 Server=19.03.5 Image:yetus/hadoop:c44943d1fc3 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12990629/HDFS-15075.004.patch |
Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle xml |
uname | Linux 6eb48bcd9bac 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | trunk / cebce0a |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_232 |
findbugs | v3.1.0-RC1 |
checkstyle | https://builds.apache.org/job/PreCommit-HDFS-Build/28647/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt |
unit | https://builds.apache.org/job/PreCommit-HDFS-Build/28647/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/28647/testReport/ |
Max. process+thread count | 4502 (vs. ulimit of 5500) |
modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/28647/console |
Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
Considering HDFS-15113 has pushed to trunk, should we continue to this improvement?
I try to rebase and upload new patch v005. Please help to review if have bandwidth. Thanks. elgoiri
Yetus is misbehaving these days....
Anyway, a couple minor comments to wrap it up:
- Let's do: public static final long DFS_DATANODE_PROCESS_COMMANDS_THRESHOLD_DEFAULT = TimeUnit.SECONDS.toMillis(2);.
- I know it was already there, but as we churning that code, let's make the LOG info message use the logger format {}.
- Any basic tests we can add at least?
Thanks elgoiri for your comments, v006 try to update following above suggestions,
a. add unit test to verify part of new metrics (which can be checked directly) but both of them.
b. update metrics document.
Thanks hexiaoqiao for the updated patch.
Good call on the doc file.
After checking more, I have a few minor comments... sorry for bringing them up late:
- The finally block in the new TestDataNodeMetrics test, should be expanded. Usually, Yetus would complain.
- Add a few basic comments to the new test (e.g., "Write into a file to trigger DN metrics").
- I know that DataNodeMetrics doesn't have javadocs, but given that we have latencies and millis in some parameters, I would make all of them called "latency" and to have a javadoc saying is milliseconds (let's do this just for the new methods).
- Let's move the substraction of the time inside the null checks all over FsDatasetImpl, there is no point doing the substraction and then not doing anything if it is null.
Thanks elgoiri for your good catches, v007 try to update the patch following suggestions. Please give another reviews. Thanks again.
Retriggered jenkins: https://builds.apache.org/job/PreCommit-HDFS-Build/29009/
+1 on HDFS-15075.007.patch.
Let's see what Jenkins says (thanks weichiu for triggering).
Great work. I wish the extra metrics are a separate jira. Looking at the jira summary I wouldn't know the bulk of the change is for adding extra metrics.
Also, the added metrics are a little similar/overlap with the per volume file IO metrics added in HDFS-10959 (which is not enabled by default).
Thanks weichiu for the comment, I think it is fair.
hexiaoqiao do you mind limitting this JIRA to fixing the metrics in BPServiceActor and create a separate JIRA for the other metrics?
Thanks weichiu,elgoiri for your suggestions, v008 try to fix about metrics in BPServiceActor only and others change will following by the next JIRA. For the other added metrics I think it is different with IO metric for each volume since existing metrics both focus on external storage IO performance, added metrics focus on hold lock times. IIUC it is also necessary for performance. FYI. Thanks again.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 46s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
trunk Compile Tests | |||
+1 | mvninstall | 19m 47s | trunk passed |
+1 | compile | 1m 1s | trunk passed |
+1 | checkstyle | 0m 50s | trunk passed |
+1 | mvnsite | 1m 5s | trunk passed |
+1 | shadedclient | 16m 22s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 52s | trunk passed |
+1 | javadoc | 0m 38s | trunk passed |
Patch Compile Tests | |||
+1 | mvninstall | 1m 3s | the patch passed |
+1 | compile | 0m 56s | the patch passed |
+1 | javac | 0m 56s | the patch passed |
+1 | checkstyle | 0m 46s | the patch passed |
+1 | mvnsite | 1m 1s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 2s | The patch has no ill-formed XML file. |
+1 | shadedclient | 14m 13s | patch has no errors when building and testing our client artifacts. |
-1 | findbugs | 2m 58s | hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
+1 | javadoc | 0m 36s | the patch passed |
Other Tests | |||
+1 | unit | 106m 58s | hadoop-hdfs in the patch passed. |
+1 | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
172m 25s |
Reason | Tests |
---|---|
FindBugs | module:hadoop-hdfs-project/hadoop-hdfs |
Redundant nullcheck of cmds, which is known to be non-null in org.apache.hadoop.hdfs.server.datanode.BPServiceActor$CommandProcessingThread.processCommand(DatanodeCommand[]) Redundant null check at BPServiceActor.java:is known to be non-null in org.apache.hadoop.hdfs.server.datanode.BPServiceActor$CommandProcessingThread.processCommand(DatanodeCommand[]) Redundant null check at BPServiceActor.java:[line 1368] |
Subsystem | Report/Notes |
---|---|
Docker | Client=19.03.8 Server=19.03.8 Image:yetus/hadoop:4454c6d14b7 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12997636/HDFS-15075.008.patch |
Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle xml |
uname | Linux 3694c1f424a2 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | trunk / d353b30 |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_242 |
findbugs | v3.1.0-RC1 |
findbugs | https://builds.apache.org/job/PreCommit-HDFS-Build/29021/artifact/out/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html |
Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/29021/testReport/ |
Max. process+thread count | 2995 (vs. ulimit of 5500) |
modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/29021/console |
Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
Hi weichiu,elgoiri, add metrics for FsDatasetImpl split from here to HDFS-15242. Please give another reviews if have bandwidth. Thanks.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 35s | Docker mode activated. |
Prechecks | |||
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
trunk Compile Tests | |||
+1 | mvninstall | 17m 41s | trunk passed |
+1 | compile | 1m 2s | trunk passed |
+1 | checkstyle | 0m 56s | trunk passed |
+1 | mvnsite | 1m 7s | trunk passed |
+1 | shadedclient | 15m 20s | branch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 38s | trunk passed |
+1 | javadoc | 0m 43s | trunk passed |
Patch Compile Tests | |||
+1 | mvninstall | 1m 1s | the patch passed |
+1 | compile | 0m 56s | the patch passed |
+1 | javac | 0m 56s | the patch passed |
+1 | checkstyle | 0m 50s | the patch passed |
+1 | mvnsite | 0m 59s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | xml | 0m 1s | The patch has no ill-formed XML file. |
+1 | shadedclient | 13m 27s | patch has no errors when building and testing our client artifacts. |
+1 | findbugs | 2m 42s | the patch passed |
+1 | javadoc | 0m 39s | the patch passed |
Other Tests | |||
-1 | unit | 91m 45s | hadoop-hdfs in the patch passed. |
+1 | asflicense | 0m 38s | The patch does not generate ASF License warnings. |
153m 5s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.hdfs.server.datanode.TestDataNodeUUID |
hadoop.hdfs.TestDFSInotifyEventInputStreamKerberized |
Subsystem | Report/Notes |
---|---|
Docker | Client=19.03.8 Server=19.03.8 Image:yetus/hadoop:4454c6d14b7 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12997664/HDFS-15075.009.patch |
Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle xml |
uname | Linux dc15d9c19882 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/patchprocess/precommit/personality/provided.sh |
git revision | trunk / cdb2107 |
maven | version: Apache Maven 3.3.9 |
Default Java | 1.8.0_242 |
findbugs | v3.1.0-RC1 |
unit | https://builds.apache.org/job/PreCommit-HDFS-Build/29023/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/29023/testReport/ |
Max. process+thread count | 4377 (vs. ulimit of 5500) |
modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/29023/console |
Powered by | Apache Yetus 0.8.0 http://yetus.apache.org |
This message was automatically generated.
Thanks for doing the split.
HDFS-15075.009.patch LGTM and it solves the concerns that weichiu brought up.
The unit tests failures aren't related.
+1
If nobody has any other concerns I'll go ahead and commit it soon.
We'll follow up on the rest in HDFS-15242.
Thanks hexiaoqiao for the fix and weichiu for the review.
Committed to trunk.
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #18090 (See https://builds.apache.org/job/Hadoop-trunk-Commit/18090/)
HDFS-15075. Remove process command timing from BPServiceActor. (inigoiri: rev cdcb77a2c5ca99502d2ac2fbf803f22463eb1343)
- (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPServiceActor.java
- (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
- (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/metrics/DataNodeMetrics.java
- (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
- (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBPOfferService.java
- (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DNConf.java
hexiaoqiao, thought here?
I am referring to:
We may want to do this timing in CommandProcessingThread now and maybe track this times in a counter.