HDFS-5914 yesterday to the HDFS-4685 branch to take care of ACL serialization after the protobuf merge.
+1 for the proposal. We have a choice between 2 possible code maintenance scenarios:
- Persist the ACL bit. Guarantee that all code paths accepting an FsPermission from the client don't trust it and don't allow it to change the persisted version. The benefit is that we don't need to do outbound translation to toggle on the ACL bit for APIs like getFileStatus. The drawback is that we need to remember to do inbound translation to maintain the persisted value of the ACL bit for APIs like setPermission.
- Do not persist the ACL bit. Guarantee that all code paths returning an FsPermission to the client toggle on the ACL bit if the inode has an AclFeature. The benefit is that we don't need to do inbound translation. The drawback is that we need to remember to do outbound translation.
HDFS-4685 branch currently implements #1, but I agree that #2 is superior, because it reduces risk. Bugs in strategy #1 could result in toggling the ACL bit on or off incorrectly, which impacts permission enforcement. Bugs in strategy #2 would only return incorrect results to a client, but would not compromise permission enforcement.
Thanks for proposing the change, Haohui.