Copilot commented on code in PR #3780:
URL: https://github.com/apache/celeborn/pull/3780#discussion_r3711845817


##########
client/src/main/java/org/apache/celeborn/client/read/CelebornInputStream.java:
##########
@@ -995,10 +995,14 @@ private boolean fillBuffer() throws IOException {
              * a FetchFailedException; and that will make the TaskContext as 
failed with shuffle fetch issues - see SPARK-19276 for more.

Review Comment:
   This comment references `ExceptionMaker.makeException`, but the interface 
method is `makeFetchFailureException`. Updating the reference avoids confusion 
when debugging or searching for the API.



##########
client/src/main/java/org/apache/celeborn/client/read/CelebornInputStream.java:
##########
@@ -995,10 +995,14 @@ private boolean fillBuffer() throws IOException {
              * a FetchFailedException; and that will make the TaskContext as 
failed with shuffle fetch issues - see SPARK-19276 for more.
              * Given this, Celeborn can wrap the FetchFailedException with our 
CelebornIOException
              */
+            String workerHost =
+                Optional.ofNullable(currentReader)
+                    .map(r -> r.getLocation().getHost())
+                    .orElse("unknown");

Review Comment:
   `currentReader.getLocation()` can be null; in that case `.map(r -> 
r.getLocation().getHost())` will throw a `NullPointerException` while handling 
the original fetch failure, masking the root cause. Map the location first (or 
use method references) so a null location safely falls back to `"unknown"`.



-- 
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]

Reply via email to