[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14545154#comment-14545154
]
Yi Liu commented on HADOOP-11938:
-
Looks good now, one nit, +1 after addressing
in TestRawCoderBase.java
{code}
Assert.fail(Encoding test with bad input passed);
{code}
We should write Encoding test with bad input should fail. You write
oppositely. Same as few other Assert.fail.
Furthermore, we need to fix the Jenkins warnings (release
audit/checkstyle/whitespace) if they are related to this patch.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-v3.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14545269#comment-14545269
]
Hadoop QA commented on HADOOP-11938:
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 14m 51s | Pre-patch HDFS-7285 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:green}+1{color} | tests included | 0m 0s | The patch appears to
include 6 new or modified test files. |
| {color:green}+1{color} | javac | 7m 34s | There were no new javac warning
messages. |
| {color:green}+1{color} | javadoc | 9m 41s | There were no new javadoc
warning messages. |
| {color:red}-1{color} | release audit | 0m 15s | The applied patch generated
1 release audit warnings. |
| {color:green}+1{color} | checkstyle | 1m 5s | There were no new checkstyle
issues. |
| {color:green}+1{color} | whitespace | 0m 11s | The patch has no lines that
end in whitespace. |
| {color:green}+1{color} | install | 1m 42s | mvn install still works. |
| {color:green}+1{color} | eclipse:eclipse | 0m 34s | The patch built with
eclipse:eclipse. |
| {color:green}+1{color} | findbugs | 1m 41s | The patch does not introduce
any new Findbugs (version 2.0.3) warnings. |
| {color:green}+1{color} | common tests | 22m 34s | Tests passed in
hadoop-common. |
| | | 60m 14s | |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12733092/HADOOP-11938-HDFS-7285-v4.patch
|
| Optional Tests | javadoc javac unit findbugs checkstyle |
| git revision | HDFS-7285 / a35936d |
| Release Audit |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6702/artifact/patchprocess/patchReleaseAuditProblems.txt
|
| hadoop-common test log |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6702/artifact/patchprocess/testrun_hadoop-common.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6702/testReport/ |
| Java | 1.7.0_55 |
| uname | Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP
PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Console output |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6702/console |
This message was automatically generated.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-v3.patch,
HADOOP-11938-HDFS-7285-v4.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14546289#comment-14546289
]
Kai Zheng commented on HADOOP-11938:
Looks all good. I will commit this unless anybody suggests otherwise.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-v3.patch,
HADOOP-11938-HDFS-7285-v4.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14543285#comment-14543285
]
Kai Zheng commented on HADOOP-11938:
Also rebased, hopefully the building results will be good. I have passed all
the related tests locally.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-v3.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14543335#comment-14543335
]
Hadoop QA commented on HADOOP-11938:
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 15m 16s | Pre-patch HDFS-7285 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:green}+1{color} | tests included | 0m 0s | The patch appears to
include 6 new or modified test files. |
| {color:green}+1{color} | javac | 7m 43s | There were no new javac warning
messages. |
| {color:green}+1{color} | javadoc | 9m 58s | There were no new javadoc
warning messages. |
| {color:red}-1{color} | release audit | 0m 16s | The applied patch generated
1 release audit warnings. |
| {color:red}-1{color} | checkstyle | 1m 5s | The applied patch generated 1
new checkstyle issues (total was 30, now 29). |
| {color:red}-1{color} | whitespace | 0m 12s | The patch has 1 line(s) that
end in whitespace. Use git apply --whitespace=fix. |
| {color:green}+1{color} | install | 1m 40s | mvn install still works. |
| {color:green}+1{color} | eclipse:eclipse | 0m 33s | The patch built with
eclipse:eclipse. |
| {color:green}+1{color} | findbugs | 1m 43s | The patch does not introduce
any new Findbugs (version 2.0.3) warnings. |
| {color:green}+1{color} | common tests | 23m 4s | Tests passed in
hadoop-common. |
| | | 61m 37s | |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12732790/HADOOP-11938-HDFS-7285-v3.patch
|
| Optional Tests | javadoc javac unit findbugs checkstyle |
| git revision | HDFS-7285 / a35936d |
| Release Audit |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6688/artifact/patchprocess/patchReleaseAuditProblems.txt
|
| checkstyle |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6688/artifact/patchprocess/diffcheckstylehadoop-common.txt
|
| whitespace |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6688/artifact/patchprocess/whitespace.txt
|
| hadoop-common test log |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6688/artifact/patchprocess/testrun_hadoop-common.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6688/testReport/ |
| Java | 1.7.0_55 |
| uname | Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP
PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Console output |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6688/console |
This message was automatically generated.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-v3.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14543076#comment-14543076
]
Yi Liu commented on HADOOP-11938:
-
Looks more better than original.
Some lines are longer than 80 chars.
*In AbstractRawErasureCoder.java*
{code}
+for (int i = pos; i buffer.remaining(); ++i) {
+ buffer.put(i, (byte) 0);
}
{code}
it should be {{buffer.limit()}} instead of remaining
And we can just use {{buffer.put((byte)0)}}
{code}
@return the buffer itself, with ZERO bytes written, remaining the original
+ * position
{code}
remaining the original position, maybe the position and limit is not
changed after the call is more clear.
use {{HadoopIllegalArgumentException}} instead of {{IllegalArgumentException}}
*in XORRawDecoder.java and XORRawEncoder.java*
{code}
inputs[i].position() + inputs[0].remaining()
{code}
Just need use inputs\[i\].limit()
*in RSRawDecoder.java and RSRawEncoder.java*
{code}
+int dataLen = inputs[0].remaining();
{code}
is it necessary? I think we don't need to pass {{dataLen}} to
{{RSUtil.GF.solveVandermondeSystem}}
*in GaloisField.java*
{code}
public void solveVandermondeSystem(int[] x, ByteBuffer[] y,
int len, int dataLen) {
{code}
As in previous comment, {{dataLen}} is unnecessary, so idx1 p.position() +
dataLen can be idx1 p.limit()
{code}
public void substitute(ByteBuffer[] p, ByteBuffer q, int x) {
-int y = 1;
+int y = 1, iIdx, oIdx;
+int len = p[0].remaining();
for (int i = 0; i p.length; i++) {
ByteBuffer pi = p[i];
- int len = pi.remaining();
- for (int j = 0; j len; j++) {
-int pij = pi.get(j) 0x00FF;
-q.put(j, (byte) (q.get(j) ^ mulTable[pij][y]));
+ for (iIdx = pi.position(), oIdx = q.position();
+ iIdx pi.position() + len; iIdx++, oIdx++) {
+int pij = pi.get(iIdx) 0x00FF;
+q.put(oIdx, (byte) (q.get(oIdx) ^ mulTable[pij][y]));
}
y = mulTable[x][y];
}
{code}
{{len}} is unnecessary.
Same for
{code}
public void remainder(ByteBuffer[] dividend, int len, int[] divisor) {
{code}
*in TestCoderBase.java*
{code}
+ private byte[] zeroChunkBytes;
..
protected void eraseDataFromChunk(ECChunk chunk) {
ByteBuffer chunkBuffer = chunk.getBuffer();
-// erase the data
-chunkBuffer.position(0);
-for (int i = 0; i chunkSize; i++) {
- chunkBuffer.put((byte) 0);
-}
+// erase the data at the position, and restore the buffer ready for reading
+// chunkSize bytes but all ZERO.
+int pos = chunkBuffer.position();
+chunkBuffer.flip();
+chunkBuffer.position(pos);
+chunkBuffer.limit(pos + chunkSize);
+chunkBuffer.put(zeroChunkBytes);
chunkBuffer.flip();
+chunkBuffer.position(pos);
+chunkBuffer.limit(pos + chunkSize);
{code}
{code}
- protected static ECChunk cloneChunkWithData(ECChunk chunk) {
+ protected ECChunk cloneChunkWithData(ECChunk chunk) {
ByteBuffer srcBuffer = chunk.getBuffer();
-ByteBuffer destBuffer;
+ByteBuffer destBuffer = allocateOutputChunkBuffer();
-byte[] bytesArr = new byte[srcBuffer.remaining()];
+byte[] bytesArr = new byte[chunkSize];
srcBuffer.mark();
srcBuffer.get(bytesArr);
srcBuffer.reset();
-if (srcBuffer.hasArray()) {
- destBuffer = ByteBuffer.wrap(bytesArr);
-} else {
- destBuffer = ByteBuffer.allocateDirect(srcBuffer.remaining());
- destBuffer.put(bytesArr);
- destBuffer.flip();
-}
+int pos = destBuffer.position();
+destBuffer.put(bytesArr);
+destBuffer.flip();
+destBuffer.position(pos);
{code}
{{destBuffer}} is still assumed to be chunkSize
Furthermore, some unnecessary flip
{code}
+ /**
+ * Convert an array of this chunks to an array of byte array.
+ * Note the chunk buffers are not affected.
+ * @param chunks
+ * @return an array of byte array
+ */
+ protected byte[][] toArrays(ECChunk[] chunks) {
+byte[][] bytesArr = new byte[chunks.length][];
+
+ByteBuffer buffer;
+for (int i = 0; i chunks.length; i++) {
+ buffer = chunks[i].getBuffer();
+ if (buffer.hasArray() buffer.position() == 0
+ buffer.remaining() == chunkSize) {
+bytesArr[i] = buffer.array();
+ } else {
+bytesArr[i] = new byte[buffer.remaining()];
+// Avoid affecting the original one
+buffer.mark();
+buffer.get(bytesArr[i]);
+buffer.reset();
+ }
+}
+
+return bytesArr;
+ }
{code}
We already have this method, use {{ECChunk.toBuffers}} ? Convert to
bytebuffer is enough? If not, should we have this method in code not only in
test?
*In TestRawCoderBase.java*
You should add more description about your tests, for example, the negative
test is for what and how you test, you also need find a good name for it.
{code}
protected
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14543129#comment-14543129
]
Kai Zheng commented on HADOOP-11938:
Thanks again [~hitliuyi] for finishing the detailed great review. It's pretty
helpful. I will update the patch accordingly today.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14543078#comment-14543078
]
Yi Liu commented on HADOOP-11938:
-
One more comment:
Add more javadoc for XOR coder, then other guys can have more better understand.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14542705#comment-14542705
]
Kai Zheng commented on HADOOP-11938:
This needs a rebase after HADOOP-11566.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14541239#comment-14541239
]
Hadoop QA commented on HADOOP-11938:
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 14m 48s | Pre-patch HDFS-7285 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:green}+1{color} | tests included | 0m 0s | The patch appears to
include 6 new or modified test files. |
| {color:green}+1{color} | javac | 7m 34s | There were no new javac warning
messages. |
| {color:green}+1{color} | javadoc | 9m 47s | There were no new javadoc
warning messages. |
| {color:red}-1{color} | release audit | 0m 14s | The applied patch generated
1 release audit warnings. |
| {color:red}-1{color} | checkstyle | 1m 6s | The applied patch generated 1
new checkstyle issues (total was 29, now 28). |
| {color:red}-1{color} | whitespace | 0m 10s | The patch has 1 line(s) that
end in whitespace. Use git apply --whitespace=fix. |
| {color:green}+1{color} | install | 1m 38s | mvn install still works. |
| {color:green}+1{color} | eclipse:eclipse | 0m 32s | The patch built with
eclipse:eclipse. |
| {color:green}+1{color} | findbugs | 1m 41s | The patch does not introduce
any new Findbugs (version 2.0.3) warnings. |
| {color:red}-1{color} | common tests | 23m 4s | Tests failed in
hadoop-common. |
| | | 60m 46s | |
\\
\\
|| Reason || Tests ||
| Failed unit tests | hadoop.io.erasurecode.coder.TestRSErasureCoder |
| | hadoop.io.erasurecode.coder.TestXORCoder |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12732217/HADOOP-11938-HDFS-7285-v2.patch
|
| Optional Tests | javadoc javac unit findbugs checkstyle |
| git revision | HDFS-7285 / 64be3d5 |
| Release Audit |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6679/artifact/patchprocess/patchReleaseAuditProblems.txt
|
| checkstyle |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6679/artifact/patchprocess/diffcheckstylehadoop-common.txt
|
| whitespace |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6679/artifact/patchprocess/whitespace.txt
|
| hadoop-common test log |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6679/artifact/patchprocess/testrun_hadoop-common.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6679/testReport/ |
| Java | 1.7.0_55 |
| uname | Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP
PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Console output |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6679/console |
This message was automatically generated.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539776#comment-14539776
]
Yi Liu commented on HADOOP-11938:
-
OK, I see. Thanks for updating the patch, I will give comments to your update
patch tomorrow.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539753#comment-14539753
]
Yi Liu commented on HADOOP-11938:
-
{quote}
Yes. XOR coder can only recover one erasure of unit
{quote}
Interesting, if so, why we need XOR coder, what's it used for? We should
remove XOR coder and then no need to maintain the code anymore.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539766#comment-14539766
]
Kai Zheng commented on HADOOP-11938:
As I mentioned above, XOR coder will not likely independently used, instead can
be utilized by other coders, like HH coder. XOR is an important primitive code,
LRC code (ref. HDFS-7345) also uses it.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-v2.patch, HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14537713#comment-14537713
]
Yi Liu commented on HADOOP-11938:
-
Some comments:
In *AbstractRawErasureCoder.java*
{code}
+ protected ByteBuffer resetOutputBuffer(ByteBuffer buffer) {
+int pos = buffer.position();
+buffer.put(zeroChunkBytes);
+buffer.position(pos);
+
+return buffer;
}
{code}
length of zeroChunkBytes could be larger than buffer.remaining(), just use a
*for* to put 0 to buffer.
{code}
protected ByteBuffer resetInputBuffer(ByteBuffer buffer) {
{code}
What's the reason we need to reset inputbuffer? Input buffers are given by
caller.
In AbstractRawErasureDecoder.java
{code}
boolean usingDirectBuffer = ! inputs[0].hasArray();
{code}
use {{inputs\[0\].isDirect();}}
{code}
@Override
public void decode(ByteBuffer[] inputs, int[] erasedIndexes, ByteBuffer[]
outputs)
@Override
public void decode(byte[][] inputs, int[] erasedIndexes, byte[][] outputs) {
{code}
We should do following check:
1. all the input have the same length besides some inputs are null, and check
output has enough space.
2. {{erasedIndexes}} matches the {{null}} position of inputs.
We should also enhance the description in RawErasureDecoder#decode to describe
more about decode/reconstruct.
{code}
+for (int i = 0; i outputs.length; ++i) {
+ buffer = outputs[i];
+ // to be ready for read dataLen bytes
+ buffer.flip();
+ buffer.position(outputOffsets[i]);
+ buffer.limit(outputOffsets[i] + dataLen);
}
{code}
This is unnecessary, remove it.
In *AbstractRawErasureEncoder.java*
{code}
boolean usingDirectBuffer = ! inputs[0].hasArray();
{code}
use {{inputs\[0\].isDirect();}}
all comments are same as in *AbstractRawErasureDecoder*
in *RSRawDecoder.java*
{code}
assert (getNumDataUnits() + getNumParityUnits() RSUtil.GF.getFieldSize());
this.errSignature = new int[getNumParityUnits()];
this.primitivePower = RSUtil.getPrimitivePower(getNumDataUnits(),
getNumParityUnits());
{code}
Why not use {{numDataUnits}} and {{numParityUnits}} directly?
In *RSRawEncoder.java*
in {{initialize}}, use {{numDataUnits}} and {{numParityUnits}} directly.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14537714#comment-14537714
]
Yi Liu commented on HADOOP-11938:
-
Will post more later.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14537768#comment-14537768
]
Kai Zheng commented on HADOOP-11938:
Hi [~hitliuyi],
Thanks for your review and the great comments. They're really helpful to make
the solid base. My following patch will address most of them. Thanks!
bq. What's the reason we need to reset inputbuffer? Input buffers are given by
caller.
Yes for now we don't need it. I got it from HADOOP-11847, there it's needed to
reset input buffers provided from reused internal buffers. I will remove it and
only add it when really necessary then.
bq. all the input have the same length besides some inputs are null
For now all the inputs shall not be null. In HADOOP-11847 some inputs can be
null to indicate not to read those units. I would really add such checks in
HADOOP-11847 because it's not trivial to do such checks. This patch would be
better to focus on the ByteBuffer supporting.
bq. We should also enhance the description in RawErasureDecoder#decode
Yes it should . I did it in HADOOP-11847 already. Could we leave it now in this
issue?
Thanks a lot!
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539254#comment-14539254
]
Yi Liu commented on HADOOP-11938:
-
In *XORRawDecoder.java*
{code}
int dataLen = inputs[0].remaining();
int erasedIdx = erasedIndexes[0];
// Process the inputs.
int iPos, oPos, iIdx, oIdx;
oPos = output.position();
for (int i = 0; i inputs.length; i++) {
// Skip the erased location.
if (i == erasedIdx) {
continue;
}
iPos = inputs[i].position();
for (iIdx = iPos, oIdx = oPos;
iIdx iPos + dataLen; iIdx++, oIdx++) {
output.put(oIdx, (byte) (output.get(oIdx) ^ inputs[i].get(iIdx)));
}
}
{code}
{{dataLen/iPos/oPos}} are not necessary. we can use buffer.limit(), position()
instead.
{code}
@Override
protected void doDecode(ByteBuffer[] inputs, int[] erasedIndexes,
ByteBuffer[] outputs) {
ByteBuffer output = outputs[0];
resetOutputBuffer(output);
int dataLen = inputs[0].remaining();
int erasedIdx = erasedIndexes[0];
// Process the inputs.
int iPos, oPos, iIdx, oIdx;
oPos = output.position();
for (int i = 0; i inputs.length; i++) {
// Skip the erased location.
if (i == erasedIdx) {
continue;
}
iPos = inputs[i].position();
for (iIdx = iPos, oIdx = oPos;
iIdx iPos + dataLen; iIdx++, oIdx++) {
output.put(oIdx, (byte) (output.get(oIdx) ^ inputs[i].get(iIdx)));
}
}
}
{code}
I wonder whether this works, I see it only decode for *output\[0\]*.
Do we ever test this? if not, we should have more test in this patch.
In *XORRawEncoder.java*
same comments as in XORRawDecoder
In *GaloisField.java*
{code}
+ByteBuffer p, prev, after;
+int pos1, idx1, pos2, idx2;
{code}
besides {{idx1/idx2}}, others are unnecessary, then the code is more clear.
Also in some other places, most of them are unnecessary, if they are only used
once or twice, we don't need declare a separate variable.
*For the tests, I want to see more tests:*
1) The length of inputs/outputs is not equal to chunksize, we can still decode.
2) Some negative test, we can catch the expected exception.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14539276#comment-14539276
]
Kai Zheng commented on HADOOP-11938:
Thanks Yi for the complete detailed review!
bq. dataLen/iPos/oPos are not necessary. we can use buffer.limit(), position()
instead.
I agree.
bq. I wonder whether this works, I see it only decode for output\[0\].
Yes. XOR coder can only recover one erasure of unit. There are tests for
recovering one unit. Please note for this limit XOR coder will not
independently used, instead can be utilized by other coders, like HH coder.
bq. besides idx1/idx2, others are unnecessary, then the code is more clear.
One reason I named the variable is to reduce the lengthy code line that
otherwise has to spread to multiple lines not naturally at all. Using the
variables would make the relevant statements/expressions very compact, also
avoiding too many nesting. Does this make sense? I'm OK not to use them if you
insist. Thanks.
bq. The length of inputs/outputs is not equal to chunksize, we can still decode.
Good idea! Per our latest discussion we need to support variable width of data
as inputs, we need to add such tests.
bq. Some negative test, we can catch the expected exception.
Yes I can do that.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14537591#comment-14537591
]
Hadoop QA commented on HADOOP-11938:
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 14m 31s | Pre-patch HDFS-7285 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:green}+1{color} | tests included | 0m 0s | The patch appears to
include 4 new or modified test files. |
| {color:green}+1{color} | javac | 7m 29s | There were no new javac warning
messages. |
| {color:green}+1{color} | javadoc | 9m 38s | There were no new javadoc
warning messages. |
| {color:red}-1{color} | release audit | 1m 6s | The applied patch generated
12 release audit warnings. |
| {color:red}-1{color} | checkstyle | 1m 5s | The applied patch generated 6
new checkstyle issues (total was 23, now 27). |
| {color:green}+1{color} | whitespace | 0m 6s | The patch has no lines that
end in whitespace. |
| {color:green}+1{color} | install | 1m 36s | mvn install still works. |
| {color:green}+1{color} | eclipse:eclipse | 0m 31s | The patch built with
eclipse:eclipse. |
| {color:green}+1{color} | findbugs | 1m 40s | The patch does not introduce
any new Findbugs (version 2.0.3) warnings. |
| {color:green}+1{color} | common tests | 22m 58s | Tests passed in
hadoop-common. |
| | | 60m 44s | |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12731851/HADOOP-11938-HDFS-7285-v1.patch
|
| Optional Tests | javadoc javac unit findbugs checkstyle |
| git revision | HDFS-7285 / d96c64c |
| Release Audit |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6583/artifact/patchprocess/patchReleaseAuditProblems.txt
|
| checkstyle |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6583/artifact/patchprocess/diffcheckstylehadoop-common.txt
|
| hadoop-common test log |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6583/artifact/patchprocess/testrun_hadoop-common.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6583/testReport/ |
| Java | 1.7.0_55 |
| uname | Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP
PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Console output |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6583/console |
This message was automatically generated.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14537548#comment-14537548
]
Kai Zheng commented on HADOOP-11938:
Erasure coder tests are also improved to simulate all kinds of ByteBuffers.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-v1.patch,
HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14533953#comment-14533953
]
Kai Zheng commented on HADOOP-11938:
This will also resolve the following test case failure. It is exposed by latest
change in erasure coder.
{noformat}
2015-05-08 18:50:06,607 WARN datanode.DataNode
(ErasureCodingWorker.java:run(386)) - Transfer failed for all targets.
2015-05-08 18:50:06,608 WARN datanode.DataNode
(ErasureCodingWorker.java:run(399)) - Failed to recover striped block:
BP-1597876081-10.239.12.51-1431082199073:blk_-9223372036854775792_1001
2015-05-08 18:50:06,609 INFO datanode.DataNode
(BlockReceiver.java:receiveBlock(826)) - Exception for
BP-1597876081-10.239.12.51-1431082199073:blk_-9223372036854775784_1001
java.io.IOException: Premature EOF from inputStream
at org.apache.hadoop.io.IOUtils.readFully(IOUtils.java:203)
at
org.apache.hadoop.hdfs.protocol.datatransfer.PacketReceiver.doReadFully(PacketReceiver.java:213)
at
org.apache.hadoop.hdfs.protocol.datatransfer.PacketReceiver.doRead(PacketReceiver.java:134)
at
org.apache.hadoop.hdfs.protocol.datatransfer.PacketReceiver.receiveNextPacket(PacketReceiver.java:109)
at
org.apache.hadoop.hdfs.server.datanode.BlockReceiver.receivePacket(BlockReceiver.java:472)
at
org.apache.hadoop.hdfs.server.datanode.BlockReceiver.receiveBlock(BlockReceiver.java:787)
at
org.apache.hadoop.hdfs.server.datanode.DataXceiver.writeBlock(DataXceiver.java:803)
at
org.apache.hadoop.hdfs.protocol.datatransfer.Receiver.opWriteBlock(Receiver.java:137)
at
org.apache.hadoop.hdfs.protocol.datatransfer.Receiver.processOp(Receiver.java:74)
at
org.apache.hadoop.hdfs.server.datanode.DataXceiver.run(DataXceiver.java:250)
at java.lang.Thread.run(Thread.java:745)
{noformat}
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, a bad optimization in below codes. It assumes the heap
buffer backed by the bytes array available for reading or writing always
starts with zero and takes the whole.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14533954#comment-14533954
]
Yi Liu commented on HADOOP-11938:
-
Thanks Kai for the catch.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, a bad optimization in below codes. It assumes the heap
buffer backed by the bytes array available for reading or writing always
starts with zero and takes the whole.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14534354#comment-14534354
]
Kai Zheng commented on HADOOP-11938:
The work around solution simply meets the assumption made by current erasure
coder implementation.
Perfect solution for this is subject to further discussion as there's no
obvious way to make both erasure coders and coder callers happy.
From coder user's point of view, it would be good to use any ByteBuffer buffer
(position 0, remaining is good);
However, from coder implementation's point of view, it should try to avoid
copying data so simply buffer.array() would be better.
Any thought? Thanks.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14534560#comment-14534560
]
Kai Zheng commented on HADOOP-11938:
I had an off-line discussion with [~hitliuyi] and was convinced that we'd
better support arbitrary ByteBuffer (with any position) for the encode/decode
input/output buffers, without causing performance loss by using two parameters
{{offset}} and {{len}} when converting on-heap ByteBuffer to an bytes array:
{code}
bytesArr = bytebuffer.array();
offset = bytebuffer.position();
len = bytebuffer.remaining();
encode(bytebuffer1) =
doEncode(bytesArr, offset, len)
{code}
As the change isn't trivial and takes time, it would be good to commit the work
around patch first in order to fix the failure sooner, and leave this issue
open for the final solution.
Thanks Yi for the great discussion!
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14534949#comment-14534949
]
Hadoop QA commented on HADOOP-11938:
\\
\\
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | pre-patch | 14m 55s | Pre-patch HDFS-7285 compilation
is healthy. |
| {color:green}+1{color} | @author | 0m 0s | The patch does not contain any
@author tags. |
| {color:red}-1{color} | tests included | 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. |
| {color:green}+1{color} | javac | 7m 31s | There were no new javac warning
messages. |
| {color:green}+1{color} | javadoc | 9m 45s | There were no new javadoc
warning messages. |
| {color:red}-1{color} | release audit | 8m 3s | The applied patch generated
394 release audit warnings. |
| {color:red}-1{color} | checkstyle | 0m 40s | The applied patch generated
1827 new checkstyle issues (total was 0, now 1823). |
| {color:red}-1{color} | whitespace | 0m 7s | The patch has 42 line(s) that
end in whitespace. Use git apply --whitespace=fix. |
| {color:green}+1{color} | install | 1m 36s | mvn install still works. |
| {color:green}+1{color} | eclipse:eclipse | 0m 32s | The patch built with
eclipse:eclipse. |
| {color:red}-1{color} | findbugs | 3m 13s | The patch appears to introduce 8
new Findbugs (version 2.0.3) warnings. |
| {color:green}+1{color} | native | 3m 15s | Pre-build of native portion |
| {color:red}-1{color} | hdfs tests | 175m 5s | Tests failed in hadoop-hdfs. |
| | | 224m 46s | |
\\
\\
|| Reason || Tests ||
| FindBugs | module:hadoop-hdfs |
| | Inconsistent synchronization of
org.apache.hadoop.hdfs.DFSOutputStream.streamer; locked 89% of time
Unsynchronized access at DFSOutputStream.java:89% of time Unsynchronized
access at DFSOutputStream.java:[line 146] |
| | Possible null pointer dereference of arr$ in
org.apache.hadoop.hdfs.server.blockmanagement.BlockInfoStripedUnderConstruction.initializeBlockRecovery(long)
Dereferenced at BlockInfoStripedUnderConstruction.java:arr$ in
org.apache.hadoop.hdfs.server.blockmanagement.BlockInfoStripedUnderConstruction.initializeBlockRecovery(long)
Dereferenced at BlockInfoStripedUnderConstruction.java:[line 206] |
| | Unread field:field be static? At ErasureCodingWorker.java:[line 251] |
| | Should
org.apache.hadoop.hdfs.server.datanode.erasurecode.ErasureCodingWorker$StripedReader
be a _static_ inner class? At ErasureCodingWorker.java:inner class? At
ErasureCodingWorker.java:[lines 914-916] |
| | Found reliance on default encoding in
org.apache.hadoop.hdfs.server.namenode.ErasureCodingZoneManager.createErasureCodingZone(String,
ECSchema):in
org.apache.hadoop.hdfs.server.namenode.ErasureCodingZoneManager.createErasureCodingZone(String,
ECSchema): String.getBytes() At ErasureCodingZoneManager.java:[line 117] |
| | Found reliance on default encoding in
org.apache.hadoop.hdfs.server.namenode.ErasureCodingZoneManager.getECZoneInfo(INodesInPath):in
org.apache.hadoop.hdfs.server.namenode.ErasureCodingZoneManager.getECZoneInfo(INodesInPath):
new String(byte[]) At ErasureCodingZoneManager.java:[line 81] |
| | Result of integer multiplication cast to long in
org.apache.hadoop.hdfs.util.StripedBlockUtil.constructInternalBlock(LocatedStripedBlock,
int, int, int, int) At StripedBlockUtil.java:to long in
org.apache.hadoop.hdfs.util.StripedBlockUtil.constructInternalBlock(LocatedStripedBlock,
int, int, int, int) At StripedBlockUtil.java:[line 84] |
| | Result of integer multiplication cast to long in
org.apache.hadoop.hdfs.util.StripedBlockUtil.planReadPortions(int, int, long,
int, int) At StripedBlockUtil.java:to long in
org.apache.hadoop.hdfs.util.StripedBlockUtil.planReadPortions(int, int, long,
int, int) At StripedBlockUtil.java:[line 204] |
| Failed unit tests | hadoop.hdfs.TestDFSPermission |
| | hadoop.fs.TestSymlinkHdfsFileContext |
| | hadoop.hdfs.TestDistributedFileSystem |
| | hadoop.hdfs.server.namenode.TestAuditLogs |
| | hadoop.hdfs.server.namenode.TestFileTruncate |
| | hadoop.fs.TestSymlinkHdfsFileSystem |
\\
\\
|| Subsystem || Report/Notes ||
| Patch URL |
http://issues.apache.org/jira/secure/attachment/12731439/HADOOP-11938-HDFS-7285-workaround.patch
|
| Optional Tests | javadoc javac unit findbugs checkstyle |
| git revision | HDFS-7285 / 12e267b |
| Release Audit |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6541/artifact/patchprocess/patchReleaseAuditProblems.txt
|
| checkstyle |
https://builds.apache.org/job/PreCommit-HADOOP-Build/6541/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt
|
| whitespace |
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14535817#comment-14535817
]
Kai Zheng commented on HADOOP-11938:
The reported issues are not relevant to the simple work around patch. The
failure case was fixed. No other test failures relevant to the patch.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
Attachments: HADOOP-11938-HDFS-7285-workaround.patch
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, caused by an optimization in below codes. It assumes
the heap buffer backed by the bytes array available for reading or writing
always starts with zero and takes the whole space.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HADOOP-11938) Fix ByteBuffer version encode/decode API of raw erasure coder
[
https://issues.apache.org/jira/browse/HADOOP-11938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14533817#comment-14533817
]
Kai Zheng commented on HADOOP-11938:
Will change existing tests to break the original assumptions and ensure the fix.
Fix ByteBuffer version encode/decode API of raw erasure coder
-
Key: HADOOP-11938
URL: https://issues.apache.org/jira/browse/HADOOP-11938
Project: Hadoop Common
Issue Type: Sub-task
Components: io
Reporter: Kai Zheng
Assignee: Kai Zheng
While investigating a test failure in {{TestRecoverStripedFile}}, one issue
in raw erasrue coder, a bad optimization in below codes. It assumes the heap
buffer backed by the bytes array available for reading or writing always
starts with zero and takes the whole.
{code}
protected static byte[][] toArrays(ByteBuffer[] buffers) {
byte[][] bytesArr = new byte[buffers.length][];
ByteBuffer buffer;
for (int i = 0; i buffers.length; i++) {
buffer = buffers[i];
if (buffer == null) {
bytesArr[i] = null;
continue;
}
if (buffer.hasArray()) {
bytesArr[i] = buffer.array();
} else {
throw new IllegalArgumentException(Invalid ByteBuffer passed, +
expecting heap buffer);
}
}
return bytesArr;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
