kfaraz commented on code in PR #17742:
URL: https://github.com/apache/druid/pull/17742#discussion_r1964777359
##########
extensions-core/hdfs-storage/src/test/java/org/apache/druid/indexing/common/tasklogs/HdfsTaskLogsTest.java:
##########
@@ -95,6 +95,19 @@ public void test_taskStatus() throws Exception
);
}
+ @Test
+ public void test_taskPayload() throws Exception
+ {
+ final File tmpDir = tempFolder.newFolder();
+ final File logDir = new File(tmpDir, "logs");
+ final File payload = new File(tmpDir, "payload.json");
+ final TaskLogs taskLogs = new HdfsTaskLogs(new
HdfsTaskLogsConfig(logDir.toString()), new Configuration());
+
+ Files.write("{}", payload, StandardCharsets.UTF_8);
Review Comment:
You may update this (and other usages in this file) to use
`Files.asCharSink(file, charset).write({})` as suggested by javadoc.
##########
extensions-core/hdfs-storage/src/main/java/org/apache/druid/storage/hdfs/tasklog/HdfsTaskLogs.java:
##########
@@ -207,6 +207,37 @@ public void killOlderThan(long timestamp) throws
IOException
}
}
}
+
+ /**
+ * Save payload into HDFS, so it can be retrieved later.
+ *
+ * @throws IOException When Druid fails to push the task payload.
+ */
Review Comment:
I don't think this javadoc is really adding any new info.
##########
extensions-core/hdfs-storage/src/main/java/org/apache/druid/storage/hdfs/tasklog/HdfsTaskLogs.java:
##########
@@ -207,6 +207,37 @@ public void killOlderThan(long timestamp) throws
IOException
}
}
}
+
+ /**
+ * Save payload into HDFS, so it can be retrieved later.
+ *
+ * @throws IOException When Druid fails to push the task payload.
+ */
+ @Override
+ public void pushTaskPayload(String taskId, File taskPayloadFile) throws
IOException
+ {
+ final Path path = getTaskPayloadFromId(taskId);
+ log.info("Pushing task payload [%s] to location [%s]", taskPayloadFile,
path);
Review Comment:
I don't think we need to log the entire payload as an info.
```suggestion
log.info("Pushing payload for task[%s] to location[%s]", taskId, path);
```
##########
extensions-core/hdfs-storage/src/main/java/org/apache/druid/storage/hdfs/tasklog/HdfsTaskLogs.java:
##########
@@ -207,6 +207,37 @@ public void killOlderThan(long timestamp) throws
IOException
}
}
}
+
+ /**
+ * Save payload into HDFS, so it can be retrieved later.
+ *
+ * @throws IOException When Druid fails to push the task payload.
+ */
+ @Override
+ public void pushTaskPayload(String taskId, File taskPayloadFile) throws
IOException
+ {
+ final Path path = getTaskPayloadFromId(taskId);
+ log.info("Pushing task payload [%s] to location [%s]", taskPayloadFile,
path);
+ pushTaskFile(path, taskPayloadFile);
+ }
+
+ /**
+ * Stream payload from HDFS for a task.
+ *
+ * @return InputStream for this taskPayload, if available.
+ * @throws IOException When Druid fails to read the task payload.
+ */
Review Comment:
not needed, no new info
##########
extensions-core/hdfs-storage/src/main/java/org/apache/druid/storage/hdfs/tasklog/HdfsTaskLogs.java:
##########
@@ -207,6 +207,37 @@ public void killOlderThan(long timestamp) throws
IOException
}
}
}
+
+ /**
+ * Save payload into HDFS, so it can be retrieved later.
+ *
+ * @throws IOException When Druid fails to push the task payload.
+ */
+ @Override
+ public void pushTaskPayload(String taskId, File taskPayloadFile) throws
IOException
+ {
+ final Path path = getTaskPayloadFromId(taskId);
+ log.info("Pushing task payload [%s] to location [%s]", taskPayloadFile,
path);
+ pushTaskFile(path, taskPayloadFile);
+ }
+
+ /**
+ * Stream payload from HDFS for a task.
+ *
+ * @return InputStream for this taskPayload, if available.
+ * @throws IOException When Druid fails to read the task payload.
+ */
+ @Override
+ public Optional<InputStream> streamTaskPayload(String taskId) throws
IOException
+ {
+ final Path path = getTaskPayloadFromId(taskId);
+ return streamTaskFile(path, 0);
+ }
+
+ private Path getTaskPayloadFromId(String taskId)
Review Comment:
```suggestion
private Path getTaskPayloadFileFromId(String taskId)
```
--
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]
