Skip to content

Conversation

@zhaoyim
Copy link
Contributor

@zhaoyim zhaoyim commented Oct 22, 2019

…er()

NOTICE

Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 1789 Docker mode activated.
_ Prechecks _
+1 dupname 0 No case conflicting files found.
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
0 mvndep 37 Maven dependency ordering for branch
+1 mvninstall 1099 trunk passed
+1 compile 203 trunk passed
+1 checkstyle 57 trunk passed
+1 mvnsite 127 trunk passed
+1 shadedclient 945 branch has no errors when building and testing our client artifacts.
+1 javadoc 118 trunk passed
0 spotbugs 169 Used deprecated FindBugs config; considering switching to SpotBugs.
+1 findbugs 298 trunk passed
_ Patch Compile Tests _
0 mvndep 14 Maven dependency ordering for patch
+1 mvninstall 111 the patch passed
+1 compile 198 the patch passed
+1 javac 198 the patch passed
-0 checkstyle 51 hadoop-hdfs-project: The patch generated 13 new + 1 unchanged - 0 fixed = 14 total (was 1)
+1 mvnsite 111 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 796 patch has no errors when building and testing our client artifacts.
+1 javadoc 112 the patch passed
-1 findbugs 142 hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
_ Other Tests _
+1 unit 125 hadoop-hdfs-client in the patch passed.
-1 unit 5200 hadoop-hdfs in the patch failed.
+1 asflicense 43 The patch does not generate ASF License warnings.
11778
Reason Tests
FindBugs module:hadoop-hdfs-project/hadoop-hdfs-client
Inconsistent synchronization of org.apache.hadoop.hdfs.DFSStripedInputStream.parityBuf; locked 44% of time Unsynchronized access at DFSStripedInputStream.java:44% of time Unsynchronized access at DFSStripedInputStream.java:[line 134]
Failed junit tests hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks
hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
Subsystem Report/Notes
Docker Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/1/artifact/out/Dockerfile
GITHUB PR #1667
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux ebe7e4af32fe 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / 6020505
Default Java 1.8.0_222
checkstyle https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/1/artifact/out/diff-checkstyle-hadoop-hdfs-project.txt
findbugs https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/1/artifact/out/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html
unit https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/1/testReport/
Max. process+thread count 4474 (vs. ulimit of 5500)
modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/1/console
versions git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 37 Docker mode activated.
_ Prechecks _
+1 dupname 0 No case conflicting files found.
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
0 mvndep 22 Maven dependency ordering for branch
+1 mvninstall 1086 trunk passed
+1 compile 202 trunk passed
+1 checkstyle 57 trunk passed
+1 mvnsite 124 trunk passed
+1 shadedclient 936 branch has no errors when building and testing our client artifacts.
+1 javadoc 118 trunk passed
0 spotbugs 170 Used deprecated FindBugs config; considering switching to SpotBugs.
+1 findbugs 298 trunk passed
_ Patch Compile Tests _
0 mvndep 14 Maven dependency ordering for patch
+1 mvninstall 113 the patch passed
+1 compile 196 the patch passed
+1 javac 196 the patch passed
-0 checkstyle 52 hadoop-hdfs-project: The patch generated 13 new + 1 unchanged - 0 fixed = 14 total (was 1)
+1 mvnsite 113 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 796 patch has no errors when building and testing our client artifacts.
+1 javadoc 111 the patch passed
+1 findbugs 307 the patch passed
_ Other Tests _
+1 unit 124 hadoop-hdfs-client in the patch passed.
-1 unit 5296 hadoop-hdfs in the patch failed.
+1 asflicense 43 The patch does not generate ASF License warnings.
10085
Reason Tests
Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes
hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
hadoop.hdfs.server.namenode.TestRedudantBlocks
Subsystem Report/Notes
Docker Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/2/artifact/out/Dockerfile
GITHUB PR #1667
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux f02eebf8c41c 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / a901405
Default Java 1.8.0_222
checkstyle https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/2/artifact/out/diff-checkstyle-hadoop-hdfs-project.txt
unit https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/2/testReport/
Max. process+thread count 4442 (vs. ulimit of 5500)
modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/2/console
versions git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@jojochuang
Copy link
Contributor

I think the patch makes sense to me.
@avijayanhwx FYI

public synchronized void unbuffer() {
closeCurrentBlockReaders();
if (curStripeBuf != null) {
curStripeBuf.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a minor issue. Won't we be doing this curStripeBuf.clear() step twice in the unbuffer method? The first time is in Line #127-129.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@avijayanhwx Thanks for review! Good point! It cleared twice, I removed the clear in the unbuffer. Could you help review again? Thanks!

Copy link
Contributor

@avijayanhwx avijayanhwx Oct 23, 2019

Choose a reason for hiding this comment

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

Thanks for the changes. I am trying to understand why the parityBuf clear step was removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@avijayanhwx Thanks for review again! The parityBuf can be cleared in method getParityBuffer() line # 133 - 140

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 35 Docker mode activated.
_ Prechecks _
+1 dupname 0 No case conflicting files found.
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
0 mvndep 14 Maven dependency ordering for branch
+1 mvninstall 1091 trunk passed
+1 compile 213 trunk passed
+1 checkstyle 56 trunk passed
+1 mvnsite 127 trunk passed
+1 shadedclient 934 branch has no errors when building and testing our client artifacts.
+1 javadoc 117 trunk passed
0 spotbugs 168 Used deprecated FindBugs config; considering switching to SpotBugs.
+1 findbugs 296 trunk passed
_ Patch Compile Tests _
0 mvndep 15 Maven dependency ordering for patch
+1 mvninstall 110 the patch passed
+1 compile 194 the patch passed
+1 javac 194 the patch passed
-0 checkstyle 50 hadoop-hdfs-project: The patch generated 13 new + 1 unchanged - 0 fixed = 14 total (was 1)
+1 mvnsite 114 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 784 patch has no errors when building and testing our client artifacts.
+1 javadoc 113 the patch passed
+1 findbugs 312 the patch passed
_ Other Tests _
+1 unit 128 hadoop-hdfs-client in the patch passed.
-1 unit 5199 hadoop-hdfs in the patch failed.
+1 asflicense 42 The patch does not generate ASF License warnings.
9984
Reason Tests
Failed junit tests hadoop.hdfs.tools.TestDFSZKFailoverController
hadoop.hdfs.TestMultipleNNPortQOP
hadoop.hdfs.server.namenode.snapshot.TestRenameWithSnapshots
Subsystem Report/Notes
Docker Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/3/artifact/out/Dockerfile
GITHUB PR #1667
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux 5aa49b74bf0b 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / a901405
Default Java 1.8.0_222
checkstyle https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/3/artifact/out/diff-checkstyle-hadoop-hdfs-project.txt
unit https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/3/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/3/testReport/
Max. process+thread count 3985 (vs. ulimit of 5500)
modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/3/console
versions git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@avijayanhwx
Copy link
Contributor

LGTM +1

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

+1

@zhaoyim
Copy link
Contributor Author

zhaoyim commented Oct 25, 2019

@jojochuang I saw you suggest change the "closeCurrentBlockReaders()" to "super.unbuffer()", just want to confirm, will I need to do the changes? Because I got the mail but can NOT find the comments in the PR. Thanks!

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 37 Docker mode activated.
_ Prechecks _
+1 dupname 0 No case conflicting files found.
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
0 mvndep 21 Maven dependency ordering for branch
+1 mvninstall 1085 trunk passed
+1 compile 199 trunk passed
+1 checkstyle 57 trunk passed
+1 mvnsite 125 trunk passed
+1 shadedclient 925 branch has no errors when building and testing our client artifacts.
+1 javadoc 118 trunk passed
0 spotbugs 167 Used deprecated FindBugs config; considering switching to SpotBugs.
+1 findbugs 298 trunk passed
_ Patch Compile Tests _
0 mvndep 14 Maven dependency ordering for patch
+1 mvninstall 113 the patch passed
+1 compile 194 the patch passed
+1 javac 194 the patch passed
-0 checkstyle 52 hadoop-hdfs-project: The patch generated 13 new + 1 unchanged - 0 fixed = 14 total (was 1)
+1 mvnsite 112 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 792 patch has no errors when building and testing our client artifacts.
+1 javadoc 111 the patch passed
+1 findbugs 306 the patch passed
_ Other Tests _
+1 unit 125 hadoop-hdfs-client in the patch passed.
-1 unit 5188 hadoop-hdfs in the patch failed.
+1 asflicense 43 The patch does not generate ASF License warnings.
9949
Reason Tests
Failed junit tests hadoop.hdfs.server.blockmanagement.TestUnderReplicatedBlocks
Subsystem Report/Notes
Docker Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/4/artifact/out/Dockerfile
GITHUB PR #1667
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux 0d5116ee4225 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / 8625265
Default Java 1.8.0_222
checkstyle https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/4/artifact/out/diff-checkstyle-hadoop-hdfs-project.txt
unit https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/4/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/4/testReport/
Max. process+thread count 4097 (vs. ulimit of 5500)
modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-1667/4/console
versions git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

@jojochuang
Copy link
Contributor

+1
yeah i initially wanted to ask you to use super.unbuffer() but it's alright. Thank you!

@jojochuang jojochuang merged commit 30db895 into apache:trunk Oct 25, 2019
asfgit pushed a commit that referenced this pull request Oct 25, 2019
…er() (#1667)

Reviewed-by: Aravindan Vijayan <[email protected]>
Reviewed-by: Wei-Chiu Chuang <[email protected]>
(cherry picked from commit 30db895)
(cherry picked from commit 9316ca1)
asfgit pushed a commit that referenced this pull request Oct 25, 2019
…er() (#1667)

Reviewed-by: Aravindan Vijayan <[email protected]>
Reviewed-by: Wei-Chiu Chuang <[email protected]>
(cherry picked from commit 30db895)
amahussein pushed a commit to amahussein/hadoop that referenced this pull request Oct 29, 2019
jojochuang pushed a commit to jojochuang/hadoop that referenced this pull request Feb 2, 2020
…er() (apache#1667)

Reviewed-by: Aravindan Vijayan <[email protected]>
Reviewed-by: Wei-Chiu Chuang <[email protected]>
(cherry picked from commit 30db895)
(cherry picked from commit 9316ca1)
(cherry picked from commit 44ca2fe)
Change-Id: I6f2a60f637080fc23b99744d86b67013504c634a
RogPodge pushed a commit to RogPodge/hadoop that referenced this pull request Mar 25, 2020
bentito pushed a commit to bentito/hadoop that referenced this pull request Dec 2, 2020
…er() (apache#1667)

Reviewed-by: Aravindan Vijayan <[email protected]>
Reviewed-by: Wei-Chiu Chuang <[email protected]>
(cherry picked from commit 30db895)
(cherry picked from commit 9316ca1)
bentito pushed a commit to bentito/hadoop that referenced this pull request Dec 3, 2020
…er() (apache#1667)

Reviewed-by: Aravindan Vijayan <[email protected]>
Reviewed-by: Wei-Chiu Chuang <[email protected]>
(cherry picked from commit 30db895)
(cherry picked from commit 9316ca1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants