TEZ-4305: Invoke hflush, only if the stream supports it #120
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This PR has dependency with apache/hadoop#2949 . Will change the pom.xml to reflect the latest Hadoop version, whenever apache/hadoop#2949 lands. |
|
💔 -1 overall
This message was automatically generated. |
|
@kishendas, we will have to rethink this change to not break users who wish to run against other versions of Hadoop. @abstractdog, we may need to consider separate branch for Hadoop 3.3. There are several API changes and dependencies that are making supporting all of Hadoop 3.x difficult |
|
💔 -1 overall
This message was automatically generated. |
Please check now. It should now work with older versions of Hadoop as well. |
|
💔 -1 overall
This message was automatically generated. |
|
Created a ticket for the unit test failure, as it seems to be unrelated -> https://issues.apache.org/jira/browse/TEZ-4306 . |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
LGTM +1, just two minor asks:
created TEZ-4307 about Hadoop 3.4.0 to track the fact that this cannot be pushed until we're not upgraded to Hadoop 3.4.0 |
|
@abstractdog Please take a look. We can land this change without waiting for Hadoop 3.4.0 upgrade, as this change will work with both Hadoop 3.4.0 and previous versions. It's just that behavior will be different based on the Hadoop version we use with Tez. |
|
💔 -1 overall
This message was automatically generated. |
I agree, the latest version is even better because StreamCapabilites is already present in older Hadoop versions nit: I was wondering if we can place a DEBUG message if hflush is skipped because it's not supported other than this nit, the patch looks good to me |
This is fine in tez, we use the same code in hive (It has to be copied to hive). And there we write one event per file on s3 since s3 does not support reading a file in which data is still being written. It will write 2 log lines per query there. |
|
@abstractdog I have added the log message. Please commit the PR, if you don't have any other feedback. |
|
💔 -1 overall
This message was automatically generated. |
I think the log message should go into an else branch...currently, it's logged every time when StreamCapabilities interface is implemented |
kishendas
force-pushed
the
TEZ-4305
branch
2 times, most recently
from
996235e
to
b388611
Compare
|
@abstractdog My bad. Got tricked by my own comment. Fixed it. Good catch. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
thanks! |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
make the change backward compatible, if Tez is used with older versions of Hadoop add a utility method do not dependent on Hadoop snapshot Add log Move log message to else block Fix indentation
|
💔 -1 overall
This message was automatically generated. |
|
@abstractdog Can you please approve and commit ? Finally tamed the checkstyle beast. |
harishjp
approved these changes
May 19, 2021
|
merged to master, thanks @kishendas for the patch and @harishjp for the review! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following exception is thrown whenever we do ProtoMessageWriter.hflush on S3, which internally calls S3ABlockOutputStream.hflush which is not supported and it throws java.lang.UnsupportedOperationException.
bdffe22d96ae [mdc@18060 class="yarn.YarnUncaughtExceptionHandler" level="ERROR" thread="HistoryEventHandlingThread"] Thread Thread[HistoryEventHandlingThread, 5,main] threw an Exception.^Mjava.lang.UnsupportedOperationException: S3A streams are not Syncable^M at org.apache.hadoop.fs.s3a.S3ABlockOutputStream.hflush(S3ABlockOutputStream.java:657)^M at org.apache.hadoop.fs.FS DataOutputStream.hflush(FSDataOutputStream.java:136)^M at org.apache.hadoop.io.SequenceFile$Writer.hflush(SequenceFile.java:1367)^M at org.apache.tez.dag.history.logging.proto.ProtoMessageWriter.hflush(ProtoMessageWr iter.java:64)^M at org.apache.tez.dag.history.logging.proto.ProtoHistoryLoggingService.finishCurrentDag(ProtoHistoryLoggingService.java:239)^M at org.apache.tez.dag.history.logging.proto.ProtoHistoryLoggingService.han dleEvent(ProtoHistoryLoggingService.java:198)^M at org.apache.tez.dag.history.logging.proto.ProtoHistoryLoggingService.loop(ProtoHistoryLoggingService.java:153)^M at java.lang.Thread.run(Thread.java:748)^M
In order to fix this issue, we should first check if the method is supported or not using StreamCapabilities, before invoking it.