Thank you for your comments, Karthik Kambatla.
Noticed there is QueueACL is mapreduce code as well that can be dropped altogether? e.g. mapred QueueManager, many parts (all of?) QueueACL etc. Can we file a follow-up JIRA to drop all of that?
I am not sure if such code in mapred.QueueMananger still works today. I prefer to clean up those mapreduce code in another JIRA.
onReload: Is there a need to lock the scheduler when setting permissions? Would it be okay to limit the synchronized block to whatever was synchronized before?
Have disscussed with Daniel Templeton, synchronized block added here is to avoid findbugs warning. Actually I will be glad to remove redundant lock here.
In setQueueAcls, we seem to initially set to default permissions and then "overwrite" it with final permissions. Is the first one necessary? I quickly looked at implementation of ConfiguredAuthorizationProvider, setPermission's semantics appear to be somewhere between append and overwrite. If it is append, may be we should change that name to addPermission?
I also feel a little confused about semantics of setPermission. However this abstract method is introduced by
YARN-3100, and I'm not sure setPermission has implemented in Ranger or Sentry. I prefer to keep setPermission here as CapacityScheduler does to keep compatibility. Maybe we could refactor it in another JIRA, (maybe could separate setPermission to setPermission, addPermission, removePermission, clearPermission). Does it make sense?
And I will update this patch soon.