cnauroth commented on code in PR #4248:
URL: https://github.com/apache/hadoop/pull/4248#discussion_r904363794
##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/output/MultipleOutputs.java:
##########
@@ -570,8 +570,14 @@ public void setStatus(String status) {
*/
@SuppressWarnings("unchecked")
public void close() throws IOException, InterruptedException {
- for (RecordWriter writer : recordWriters.values()) {
- writer.close(context);
- }
+ recordWriters.values().parallelStream().forEach(writer -> {
Review Comment:
I'm concerned that this could have unintended side effects for callers,
because it changes the error contract. Errors during `close()` that were
formerly visible as a checked `IOException` or `InterruptedException` now
become an unchecked `RuntimeException`. In the case of thread interruption, the
interrupt now occurs on the background thread with no propagation of
interrupted status back up to the coordinating thread.
Unfortunately, `parallelStream()` with a lambda puts us down this path. It
would be more code, but directly managing a `ThreadPoolExecutor` would give you
the chance to preserve the contract by unwrapping checked exceptions from the
`Future` and propagating.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
