-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MAPREDUCE-7469. NNBench createControlFiles should use thread pool to improve performance. #6463
Conversation
💔 -1 overall
This message was automatically generated. |
…mprove performance.
@LiuGuH after a quick review, it seems reasonable to me. I will take a closer look later. |
try { | ||
list.get(i).get(); | ||
} catch (InterruptedException | ExecutionException e) { | ||
LOG.error("Creating control files Error."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOG.error("Creating control files Error.", e);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed. Thanks
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
LGTM |
} | ||
} | ||
|
||
executorService.shutdown(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are shutting down the service, should we spawn executorService in this method only instead of creating at object creation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks.
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the comment. LGTM!
@slfan1989 , hello, sir, can we merge this , thanks. |
@LiuGuH Sorry for the late reply, we need to trigger compilation again. If there is no issue, the PR will be merged. |
@slfan1989 Sorry for the late reply because I took a long vacation. Trigger compilation now. Thanks |
🎊 +1 overall
This message was automatically generated. |
Description of PR
JIRA: MAPREDUCE-7469 NNBench createControlFiles should use thread pool to improve performance.
How was this patch tested?
Add test case for this.