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/10/10 12:13:15 UTC
[GitHub] [ozone] sodonnel opened a new pull request, #3816: HDDS-7303. EC: ECBlockReconstructedStripeInputStream should set initialized false on re-init
sodonnel opened a new pull request, #3816:
URL: https://github.com/apache/ozone/pull/3816
## What changes were proposed in this pull request?
In ECBlockReconstructedStripeInputStream, when an exception occurs reading a block, the code calls the `init()` method to setup the missing indexes and buffers.
If an InsufficientLocations exception is thrown part way through that method, the class ends up partly re-initialized. If something then ignores / handles the InsufficientLocations and tries to call read again, it can cause strange results. In one case, we get an illegalArgumentException, which I think is related to the above:
```
Caused by: java.lang.ArrayIndexOutOfBoundsException: 1
at org.apache.hadoop.ozone.client.io.ECBlockReconstructedStripeInputStream.assignBuffers(ECBlockReconstructedStripeInputStream.java:289)
at org.apache.hadoop.ozone.client.io.ECBlockReconstructedStripeInputStream.read(ECBlockReconstructedStripeInputStream.java:360)
at org.apache.hadoop.ozone.client.io.ECBlockReconstructedStripeInputStream.readStripe(ECBlockReconstructedStripeInputStream.java:345)
at org.apache.hadoop.ozone.client.io.ECBlockReconstructedInputStream.readStripe(ECBlockReconstructedInputStream.java:214)
at org.apache.hadoop.ozone.client.io.ECBlockReconstructedInputStream.readAndSeekStripe(ECBlockReconstructedInputStream.java:198)
at org.apache.hadoop.ozone.client.io.ECBlockReconstructedInputStream.seek(ECBlockReconstructedInputStream.java:192)
at org.apache.hadoop.ozone.client.io.ECBlockInputStreamProxy.seek(ECBlockInputStreamProxy.java:224)
at org.apache.hadoop.ozone.client.io.KeyInputStream.seek(KeyInputStream.java:340)
at org.apache.hadoop.fs.ozone.OzoneFSInputStream.seek(OzoneFSInputStream.java:78)
at org.apache.hadoop.fs.FSInputStream.read(FSInputStream.java:85)
at org.apache.hadoop.fs.FSInputStream.readFully(FSInputStream.java:124)
at org.apache.hadoop.fs.FSDataInputStream.readFully(FSDataInputStream.java:116)
at org.apache.orc.impl.RecordReaderUtils$DefaultDataReader.readStripeFooter(RecordReaderUtils.java:273)
at org.apache.orc.impl.RecordReaderImpl.readStripeFooter(RecordReaderImpl.java:308)
at org.apache.orc.impl.RecordReaderImpl.beginReadStripe(RecordReaderImpl.java:1131)
at org.apache.orc.impl.RecordReaderImpl.readStripe(RecordReaderImpl.java:1093)
at org.apache.orc.impl.RecordReaderImpl.advanceStripe(RecordReaderImpl.java:1261)
at org.apache.orc.impl.RecordReaderImpl.advanceToNextRow(RecordReaderImpl.java:1296)
at org.apache.orc.impl.RecordReaderImpl.nextBatch(RecordReaderImpl.java:1332)
at org.apache.hadoop.hive.ql.io.orc.RecordReaderImpl.nextBatch(RecordReaderImpl.java:157)
at org.apache.hadoop.hive.ql.io.orc.VectorizedOrcAcidRowBatchReader$1.next(VectorizedOrcAcidRowBatchReader.java:175)
at org.apache.hadoop.hive.ql.io.orc.VectorizedOrcAcidRowBatchReader$1.next(VectorizedOrcAcidRowBatchReader.java:171)
at org.apache.hadoop.hive.ql.io.orc.VectorizedOrcAcidRowBatchReader.next(VectorizedOrcAcidRowBatchReader.java:871)
... 26 more
```
We should simply set initialized to false at the beginning of init and set it to try at the end when the full init method has completed.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7303
## How was this patch tested?
Existing test cover 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
[GitHub] [ozone] sodonnel commented on pull request #3816: HDDS-7303. EC: ECBlockReconstructedStripeInputStream should set initialized false on re-init
Posted by GitBox <gi...@apache.org>.
sodonnel commented on PR #3816:
URL: https://github.com/apache/ozone/pull/3816#issuecomment-1277840060
> Quick question: You mean application ignores exception and call read again?
Yes, that is a way to cause this problem. Whether or not it is the only way, I don't know.
--
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] adoroszlai commented on pull request #3816: HDDS-7303. EC: ECBlockReconstructedStripeInputStream should set initialized false on re-init
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3816:
URL: https://github.com/apache/ozone/pull/3816#issuecomment-1279680225
Thanks @sodonnel for the fix, @myskov, @umamaheswararao for reviewing 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] umamaheswararao commented on pull request #3816: HDDS-7303. EC: ECBlockReconstructedStripeInputStream should set initialized false on re-init
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3816:
URL: https://github.com/apache/ozone/pull/3816#issuecomment-1277813411
> If something then ignores / handles the InsufficientLocations and tries to call read again
Quick question: You mean application ignores exception and call read again?
--
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] adoroszlai merged pull request #3816: HDDS-7303. EC: ECBlockReconstructedStripeInputStream should set initialized false on re-init
Posted by GitBox <gi...@apache.org>.
adoroszlai merged PR #3816:
URL: https://github.com/apache/ozone/pull/3816
--
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 a diff in pull request #3816: HDDS-7303. EC: ECBlockReconstructedStripeInputStream should set initialized false on re-init
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3816:
URL: https://github.com/apache/ozone/pull/3816#discussion_r991318809
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockReconstructedStripeInputStream.java:
##########
@@ -212,12 +212,14 @@ public synchronized void setRecoveryIndexes(Collection<Integer> indexes) {
}
private void init() throws InsufficientLocationsException {
+ initialized = false;
if (decoder == null) {
decoder = CodecUtil.createRawDecoderWithFallback(getRepConfig());
}
if (!hasSufficientLocations()) {
- throw new InsufficientLocationsException("There are insufficient " +
- "datanodes to read the EC block");
+ String msg = "There are insufficient datanodes to read the EC block";
+ LOG.debug(msg);
Review Comment:
Generally the caller should log the exception higher up the stack. If you log at the source of the exception, you generally end up logging it twice. That is the reason I made this a debug log, but also because we are not certain this fix is the root cause of the problem we have seen, so it would be good to have the debug log incase we see the problem again.
--
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] myskov commented on a diff in pull request #3816: HDDS-7303. EC: ECBlockReconstructedStripeInputStream should set initialized false on re-init
Posted by GitBox <gi...@apache.org>.
myskov commented on code in PR #3816:
URL: https://github.com/apache/ozone/pull/3816#discussion_r991282270
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockReconstructedStripeInputStream.java:
##########
@@ -212,12 +212,14 @@ public synchronized void setRecoveryIndexes(Collection<Integer> indexes) {
}
private void init() throws InsufficientLocationsException {
+ initialized = false;
if (decoder == null) {
decoder = CodecUtil.createRawDecoderWithFallback(getRepConfig());
}
if (!hasSufficientLocations()) {
- throw new InsufficientLocationsException("There are insufficient " +
- "datanodes to read the EC block");
+ String msg = "There are insufficient datanodes to read the EC block";
+ LOG.debug(msg);
Review Comment:
sounds more like LOG.error than LOG.debug
--
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