You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/02/21 08:12:44 UTC
[GitHub] [ozone] cchenax opened a new pull request #3118: add padding problem
cchenax opened a new pull request #3118:
URL: https://github.com/apache/ozone/pull/3118
## What changes were proposed in this pull request?
when add padding,if databuffers are
buffer 0 pos is 0 buffer limit is 1048576 buffer capacity is 1048576
buffer 1 pos is 0 buffer limit is 354944 buffer capacity is 1048576
buffer 2 pos is 0 buffer limit is 354944 buffer capacity is 1048576
this situation, the buffercheck will error,so we should use the maxSize to limit the buffer
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6353
## How was this patch tested?
ci
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] umamaheswararao commented on pull request #3118: add padding problem
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1047931622
@cchenax Thanks for reporting.
Yes, I agree with other reviews here. I did not really understand whats the issue with the existing code. It would be helpful if a test demonstrates the issue.
Please note why the existing loop starts with 1 was the fact that, we will never add any padding to first buffer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng closed pull request #3118: HDDS-6353. EC: Add padding problem
Posted by GitBox <gi...@apache.org>.
guihecheng closed pull request #3118:
URL: https://github.com/apache/ozone/pull/3118
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] cchenax commented on pull request #3118: add padding problem
Posted by GitBox <gi...@apache.org>.
cchenax commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1046582696
@umamaheswararao could you review it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] cchenax commented on pull request #3118: add padding problem
Posted by GitBox <gi...@apache.org>.
cchenax commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1050636165
> When you get the error:
>
> ```
> Error: org.apache.hadoop.HadoopIllegalArgumentException: Invalid buffer, remaining is 354944not of length 1048576
> at org.apache.ozone.erasurecode.rawcoder.ByteBufferEncodingState.checkBuffers(ByteBufferEncodingState.java:101)
> ```
>
> Is there a full stack trace logged anywhere, so we can see the the calls leading up to this problem?
>
> I am not sure if the problem is an issue with the padding, rather than some other issue that is not setting the initial buffer capacity correctly.
maybe, but I think it should use the max length to limit the buffer.if capacity is 1048576, but the databuffer is 1050000, so the parityCellSize is 1424, so when addpadding,it limit 1424, so I think it is a problem
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on pull request #3118: add padding problem
Posted by GitBox <gi...@apache.org>.
sodonnel commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1047693260
From the description, it is not clear to me how this problem occurs or what the problem is. Can you please explain how this can happen or be reproduced? All buffers should have the same limit and capacity, so I am not sure how data buffers 1 and 2 can have a smaller limit than buffer zero?
Also, can we add a unit test demonstrating the issue without the fix in place?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kaijchen commented on a change in pull request #3118: add padding problem
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#discussion_r811535884
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
##########
@@ -558,13 +558,17 @@ private void handleStripeFailure(int lastStripeSize,
private void addPadding(int parityCellSize) {
ByteBuffer[] buffers = ecChunkBufferCache.getDataBuffers();
-
- for (int i = 1; i < numDataBlks; i++) {
- final int position = buffers[i].position();
- assert position <= parityCellSize : "If an internal block is smaller"
- + " than parity block, then its last cell should be small than last"
- + " parity cell";
- padBufferToLimit(buffers[i], parityCellSize);
+ int maxSize = 0;
Review comment:
It could be simplified to this.
```java
int maxSize = parityCellSize;
for (int i = 0; i < numDataBlks; i++) {
maxSize = Math.max(maxSize, buffers[i].position());
}
```
But parityCellSize shouldn't be smaller than position of any buffers,
because `lastStripeSize` is the sum of all positions.
https://github.com/apache/ozone/blob/60a41bc85e1657344a2b2029ad21e7fab741b711/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java#L518-L520
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on pull request #3118: add padding problem
Posted by GitBox <gi...@apache.org>.
sodonnel commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1050719777
> maybe, but I think it should use the max length to limit the buffer.if capacity is 1048576, but the databuffer is 1050000, so the parityCellSize is 1424, so when addpadding,it limit 1424, so I think it is a problem
We set the buffer limits to equal the parity cell size before doing the encoding. If we have a partial stripe as the last stripe in the block, where:
data_1: 1000 bytes
data_2: 0
data_3: 0
Then the parity only needs to be 1000 bytes long. Rather than pad data_1, 2 and 3 out to 1MB, we simply set all the limits to 1000, then pad data_2 and 3 with zeros to the limit of 1000. Then we generate the parity, where each of them will be 1000 long.
When this is written down to the datanodes, we do not store the padding. data_2 and data_3 will not get anything for this write. data_1 and parity_1 and parity_2 will get 1000 bytes.
Another scenario, is a small key of only 1000 bytes. It works much the same, only nothing will get written to data_2 and data_3 at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] guihecheng commented on pull request #3118: HDDS-6353. EC: Add padding problem
Posted by GitBox <gi...@apache.org>.
guihecheng commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1058051679
This problem is not reproduced anymore, so I'll close it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kaijchen commented on pull request #3118: add padding problem
Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1049644360
Hi @cchenax, I have run TestDFSIO on b9adbcd89ad84d71e73ddff26f4e039c6aeba499 and cannot reproduce the issue.
Except the NotImplementedException from flush(), for which I have fired a JIRA HDDS-6372(#3133).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kaijchen commented on pull request #3118: HDDS-6353. EC: Add padding problem
Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1053981129
@cchenax note `parityCellSize` is `min(lastStripeSize, ecChunkSize)`.
https://github.com/apache/ozone/blob/9e3cf98a30dbf16508b97aba2d8c0324c378ff87/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java#L496-L498
`lastStripeSize` equals to the sum of all data blocks in the last stripe, which in your example is 1050.
https://github.com/apache/ozone/blob/9e3cf98a30dbf16508b97aba2d8c0324c378ff87/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java#L477
https://github.com/apache/ozone/blob/9e3cf98a30dbf16508b97aba2d8c0324c378ff87/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java#L561-L568
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] cchenax edited a comment on pull request #3118: HDDS-6353. EC: Add padding problem
Posted by GitBox <gi...@apache.org>.
cchenax edited a comment on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1053957447
> > maybe, but I think it should use the max length to limit the buffer.if capacity is 1048576, but the databuffer is 1050000, so the parityCellSize is 1424, so when addpadding,it limit 1424, so I think it is a problem
>
> We set the buffer limits to equal the parity cell size before doing the encoding. If we have a partial stripe as the last stripe in the block, where:
>
> data_1: 1000 bytes data_2: 0 data_3: 0
>
> Then the parity only needs to be 1000 bytes long. Rather than pad data_1, 2 and 3 out to 1MB, we simply set all the limits to 1000, then pad data_2 and 3 with zeros to the limit of 1000. Then we generate the parity, where each of them will be 1000 long.
>
> When this is written down to the datanodes, we do not store the padding. data_2 and data_3 will not get anything for this write. data_1 and parity_1 and parity_2 will get 1000 bytes.
>
> Another scenario, is a small key of only 1000 bytes. It works much the same, only nothing will get written to data_2 and data_3 at all.
I know what you mean
each chunksize 1000bytes
I mean if the data len is 1050 byte
data_1: 1000 bytes
data_2: 50 bytes
data_3: 0 bytes
so when add padding
the data_2 and data_3 will add padding to 50bytes
I think this is a problem, because of it not use the longest size to limit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] cchenax edited a comment on pull request #3118: HDDS-6353. EC: Add padding problem
Posted by GitBox <gi...@apache.org>.
cchenax edited a comment on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1053957447
> > maybe, but I think it should use the max length to limit the buffer.if capacity is 1048576, but the databuffer is 1050000, so the parityCellSize is 1424, so when addpadding,it limit 1424, so I think it is a problem
>
> We set the buffer limits to equal the parity cell size before doing the encoding. If we have a partial stripe as the last stripe in the block, where:
>
> data_1: 1000 bytes data_2: 0 data_3: 0
>
> Then the parity only needs to be 1000 bytes long. Rather than pad data_1, 2 and 3 out to 1MB, we simply set all the limits to 1000, then pad data_2 and 3 with zeros to the limit of 1000. Then we generate the parity, where each of them will be 1000 long.
>
> When this is written down to the datanodes, we do not store the padding. data_2 and data_3 will not get anything for this write. data_1 and parity_1 and parity_2 will get 1000 bytes.
>
> Another scenario, is a small key of only 1000 bytes. It works much the same, only nothing will get written to data_2 and data_3 at all.
I know what you mean
each chunksize 1000bytes
I mean if the data len is 1050 byte
data_1: 1000 bytes
data_2: 50 bytes
data_3: 0 bytes
so when add padding
the data_2 and data_3 will add padding to 50bytes
I think this is a problem, because of it not use the longest size of data_1 and data_2 and data_3 to limit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] cchenax commented on pull request #3118: HDDS-6353. EC: Add padding problem
Posted by GitBox <gi...@apache.org>.
cchenax commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1053957447
> > maybe, but I think it should use the max length to limit the buffer.if capacity is 1048576, but the databuffer is 1050000, so the parityCellSize is 1424, so when addpadding,it limit 1424, so I think it is a problem
>
> We set the buffer limits to equal the parity cell size before doing the encoding. If we have a partial stripe as the last stripe in the block, where:
>
> data_1: 1000 bytes data_2: 0 data_3: 0
>
> Then the parity only needs to be 1000 bytes long. Rather than pad data_1, 2 and 3 out to 1MB, we simply set all the limits to 1000, then pad data_2 and 3 with zeros to the limit of 1000. Then we generate the parity, where each of them will be 1000 long.
>
> When this is written down to the datanodes, we do not store the padding. data_2 and data_3 will not get anything for this write. data_1 and parity_1 and parity_2 will get 1000 bytes.
>
> Another scenario, is a small key of only 1000 bytes. It works much the same, only nothing will get written to data_2 and data_3 at all.
I know what you mean
each chunksize 1000bytes
I mean if the data len is 1050 byte
data_1: 1000 bytes
data_2: 50 bytes
data_3: 0 bytes
so when add padding
the data_2 and data_3 will add padding to 50bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on pull request #3118: add padding problem
Posted by GitBox <gi...@apache.org>.
sodonnel commented on pull request #3118:
URL: https://github.com/apache/ozone/pull/3118#issuecomment-1048650890
When you get the error:
```
Error: org.apache.hadoop.HadoopIllegalArgumentException: Invalid buffer, remaining is 354944not of length 1048576
at org.apache.ozone.erasurecode.rawcoder.ByteBufferEncodingState.checkBuffers(ByteBufferEncodingState.java:101)
```
Is there a full stack trace logged anywhere, so we can see the the calls leading up to this problem?
I am not sure if the problem is an issue with the padding, rather than some other issue that is not setting the initial buffer capacity correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org