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/01/11 12:06:26 UTC
[GitHub] [ozone] sadanand48 opened a new pull request #2978: [Ozone-Streaming] HDDS-6139. Fix incorrect computation of totalAckDataLength.
sadanand48 opened a new pull request #2978:
URL: https://github.com/apache/ozone/pull/2978
## What changes were proposed in this pull request?
In the current streaming code we maintain only one bufferList for retry. Buffers in the list are removed only during watchForCommit. So there might occur a case if putBlock is followed by another putBlock the commitInfoMap has the same list for both indexes which will cause incorrect computation of totalAckDatalength which is used during retry
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6139
## How was this patch tested?
unit test
--
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] szetszwo commented on a change in pull request #2978: [Ozone-Streaming] HDDS-6139. Fix incorrect computation of totalAckDataLength.
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #2978:
URL: https://github.com/apache/ozone/pull/2978#discussion_r783649225
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -392,7 +398,8 @@ private void executePutBlock(boolean close,
final List<StreamBuffer> byteBufferList;
if (!force) {
Preconditions.checkNotNull(bufferList);
- byteBufferList = bufferList;
+ byteBufferList = buffersForPutBlock;
+ buffersForPutBlock = null;
Preconditions.checkNotNull(byteBufferList);
Review comment:
@sadanand48 , there is a NPE
```
java.lang.NullPointerException
at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:880)
at org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput.executePutBlock(BlockDataStreamOutput.java:403)
at org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput.handleFlush(BlockDataStreamOutput.java:489)
at org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput.close(BlockDataStreamOutput.java:509)
at org.apache.hadoop.ozone.client.io.BlockDataStreamOutputEntry.close(BlockDataStreamOutputEntry.java:122)
at org.apache.hadoop.ozone.client.io.KeyDataStreamOutput.handleStreamAction(KeyDataStreamOutput.java:460)
...
```
It seems the preconditions is incorrect.
```
Preconditions.checkNotNull(byteBufferList);
```
Please take a look. Thanks.
--
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] bshashikant merged pull request #2978: [Ozone-Streaming] HDDS-6139. Fix incorrect computation of totalAckDataLength.
Posted by GitBox <gi...@apache.org>.
bshashikant merged pull request #2978:
URL: https://github.com/apache/ozone/pull/2978
--
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] bshashikant commented on pull request #2978: [Ozone-Streaming] HDDS-6139. Fix incorrect computation of totalAckDataLength.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2978:
URL: https://github.com/apache/ozone/pull/2978#issuecomment-1011819487
Thanks @sadanand48 for the contribution.
--
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] szetszwo commented on a change in pull request #2978: [Ozone-Streaming] HDDS-6139. Fix incorrect computation of totalAckDataLength.
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #2978:
URL: https://github.com/apache/ozone/pull/2978#discussion_r782933872
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -180,6 +182,7 @@ public BlockDataStreamOutput(
checksum = new Checksum(config.getChecksumType(),
config.getBytesPerChecksum());
metrics = XceiverClientManager.getXceiverClientMetrics();
+ buffersForPutBlock = new ArrayList<>();
Review comment:
Remove this line. The list will be created in writeChunk(..).
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestBlockDataStreamOutput.java
##########
@@ -256,4 +257,22 @@ public void testMinPacketSize() throws Exception {
validateData(keyName, dataString.concat(dataString).getBytes(UTF_8));
}
+ @Test
+ public void testTotalAckDataLength() throws Exception {
+ int dataLength = 400;
+ String keyName = getKeyName();
+ OzoneDataStreamOutput key = createKey(
+ keyName, ReplicationType.RATIS, 0);
+ byte[] data =
+ ContainerTestHelper.getFixedLengthString(keyString, dataLength)
+ .getBytes(UTF_8);
+ KeyDataStreamOutput keyDataStreamOutput =
+ (KeyDataStreamOutput) key.getByteBufStreamOutput();
+ BlockDataStreamOutputEntry stream =
+ keyDataStreamOutput.getStreamEntries().get(0);
+ key.write(ByteBuffer.wrap(data));
+ key.close();
+ Assert.assertTrue(stream.getTotalAckDataLength()==dataLength);
Review comment:
Use assertEquals, i.e.
```
Assert.assertEquals(dataLength, stream.getTotalAckDataLength());
```
--
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] sadanand48 commented on a change in pull request #2978: [Ozone-Streaming] HDDS-6139. Fix incorrect computation of totalAckDataLength.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2978:
URL: https://github.com/apache/ozone/pull/2978#discussion_r783762863
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -392,7 +398,8 @@ private void executePutBlock(boolean close,
final List<StreamBuffer> byteBufferList;
if (!force) {
Preconditions.checkNotNull(bufferList);
- byteBufferList = bufferList;
+ byteBufferList = buffersForPutBlock;
+ buffersForPutBlock = null;
Preconditions.checkNotNull(byteBufferList);
Review comment:
Created PR #2984 to address this
--
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