virajjasani commented on PR #5300:
URL: https://github.com/apache/hadoop/pull/5300#issuecomment-1386270795

   Thanks for the reviews @cnauroth @sodonnel.
   
   > which will result in the image failing to load and the NN aborting, so its 
an exception that we really don't expect to happen.
   
   That is correct. As such this is going to lead to failure eventually. The 
only reason I came across this sometime back was due to profiling of a 
purposeful failure asserting test. We would like to use this parallelism of 
inodes loading with hadoop 3 upgrades (still running hadoop 2 for majority 
clusters), and hence running some tests around this.
   
   
   > Can the code be simplifed to this?
   > final List<IOException> exceptions = Collections.synchronizedList(new 
ArrayList<>());
   
   > Using `Collections.synchronizedList` does seem simpler or synchronizing on 
the exceptions object rather than having a separate lock object probably makes 
sense to simplify this change further.
   
   Sounds good, thanks.


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

Reply via email to