autumnust commented on a change in pull request #3180:
URL: https://github.com/apache/incubator-gobblin/pull/3180#discussion_r547496790
##########
File path:
gobblin-core/src/main/java/org/apache/gobblin/writer/PartitionedDataWriter.java
##########
@@ -235,18 +239,24 @@ private boolean
isDataWriterWatermarkCapable(DataWriter<D> dataWriter) {
@Override
public void writeEnvelope(RecordEnvelope<D> recordEnvelope) throws
IOException {
try {
- DataWriter<D> writer =
getDataWriterForRecord(recordEnvelope.getRecord());
+ GenericRecord partition =
getPartitionForRecord(recordEnvelope.getRecord());
+ DataWriter<D> writer = this.partitionWriters.get(partition);
+ long startTime = System.currentTimeMillis();
writer.writeEnvelope(recordEnvelope);
- } catch (ExecutionException ee) {
+ long timeForWriting = System.currentTimeMillis() - startTime;
+ // If the write take a long time, which is 1/3 of cache expiration time,
we fail the writer to avoid data loss
+ // and further slowness on the same HDFS block
+ if (timeForWriting > this.writeTimeoutInterval ) {
+ throw new TimeoutException(StringFormatter.format("Write record took
%s ms, but threshold is %s ms",
Review comment:
If you will need to catch the `TimeoutException` later again and
re-throw as IOE, why not directly throw IOE ?
##########
File path:
gobblin-core/src/main/java/org/apache/gobblin/writer/PartitionedDataWriter.java
##########
@@ -17,6 +17,7 @@
package org.apache.gobblin.writer;
+import com.sun.javafx.binding.StringFormatter;
Review comment:
`String.format` should serve the same purpose?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
