steveloughran commented on code in PR #2991:
URL: https://github.com/apache/parquet-java/pull/2991#discussion_r1717303161
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/util/HadoopPositionOutputStream.java:
##########
@@ -62,7 +64,9 @@ public void flush() throws IOException {
@Override
public void close() throws IOException {
try (FSDataOutputStream fdos = wrapped) {
- fdos.hflush();
+ if (fdos.hasCapability("hflush")) {
Review Comment:
1. adding a delegating `hasCapability()` might be handy
2. consider what to do on a sync failure. close() will sill be invoked but
any exception thrown would probably be from the hflush() failure.
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/util/HadoopPositionOutputStream.java:
##########
@@ -51,7 +51,9 @@ public void write(byte[] b, int off, int len) throws
IOException {
}
public void sync() throws IOException {
- wrapped.hsync();
+ if (wrapped.hasCapability("hsync")) {
+ wrapped.hsync();
Review Comment:
hsync is actually really expensive. even on hdfs you shouldn't be calling it
as it doesn't return until the data has been replicated and persisted.
S3A tells you off because some apps (hbase etc) really rely on sync to
commit work. there's switch to actually fail...turning that on let us find
which bits of code actually expected it to work.
Ideally code shouldn't be using outside of work whey want to be 100%
confident data is written.
My IDE doesn't show any uses of the method...is there some through
reflection?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
