With all due respect, I think that something in your behavior is inappropriate:
- You were never involved in this issue; still you gave yourself the liberty to make it a sub issue of your supported
MAPREDUCE-2454 issue, without consulting anyone.
- This is especially inappropriate since
MAPREDUCE-2454 is disputable and has its acceptance problems regardless of my issue. Hence, its acceptance problems will affect my issue.
- Your justification "As all this JIRAs are small, I think we'll be able to move fast with all of them." is inappropriate since you actually created a linkage that will surely postpone my issue instead of leaving each issue to progress at its own pace!
- It is not the first time that the persons behind
MAPREDUCE-2454 try to disturb this JIRA issue.
Apparently, I don't have the privileges to break this "sub task" linkage; hence, I am asking that you or someone else will do it.
I am welcoming any comment coming from a professional place with the simple target of making Hadoop better. Having that said, I feel that the way you blitzed my patch with any possible patty comment, sometime with disputable claims, just before the patch is about to be accepted – is unfair, unprofessional and unfriendly. Especially considering your complete silence since this JIRA issue has started.
I am not sure that commenting in a blitz way will increase the quality of hadoop. For example:
"Checking for shuffleConsumerPlugin != null before closing it seems redundant, you would have never got there if shufflePlugin is NULL."
This is your mistake (I'll reach there in case isLocal == true). There is no option to remove the nullity check!
"Visibility annotations for the ShuffleConsumerPlugin, ShuffleContext, should be Unstable"
I think it is inappropriate to declare plugin interface as "Unstable", since it must stay stable for 3rd party vendors.
— --- — ---
Personally, I have no problem to implement all the rest of your comments. It should be very easy for me. Still, I am raising few points for consideration regarding your following comments:
"The Shuffle class should be renamed to DefaultShuffle."
"The ShuffleConsumerPlugin should be renamed to Shuffle."
I chose the term 'ShuffleConsumerPlugin' and not something like 'Shuffle', because it clarifies that we are in a plugin of ShuffleConsumer, rather than a builtin ShuffleProvider/ShuffleHandler. Also, I didn't take the liberty to rename core classes of Hadoop.
"ShuffleConsumerPlugin, getShuffleConsumerPlugin() method is not required, instead use the ReflectionUtils directly in the ReducerTask class."
Here, I only followed existing convention of Hadoop as shown in ResourceCalculatorPlugin.getResourceCalculatorPlugin(). Personally, I'll be glad to follow your advice, and even to go one step further and make ShuffleConsumerPlugin an interface instead of AbstractClass.
"use 'mapreduce.job.reduce.shuffle.class' to be consistent with
Here I chose 'mapreduce.shuffle…', since I think it is consistent with the current convention in hadoop-3 configuration.
— --- — ---
I can tell you that Arun & Todd didn't make it easy for me with their requests from this patch so far. Still, I understand, respect and accept all their comments. I am sure that everyone involved only want the best for Hadoop.
I suggest we hear Arun's consideration and move forward with the patch in the best professional way.
I think you are very familiar with both Hadoop/MapReduce and this JIRA issue since its inception. You are also well familiar and involved with
MAPREDUCE-2454. It is also safe to say you know Alejandro and Asokan better than you know me. I believe everyone involved will agree that your sole interest is Hadoop's quality. I am asking you and everyone else to help progressing here.