STORM-3862 HdfsBlobStoreImpl should check permission after mkdirs

[skysiders]

STORM-3862 HdfsBlobStoreImpl should check permission after mkdirs

What is the purpose of the change

HdfsBlobStoreImpl and HdfsBlobStoreFile will create directory with 700 permission, we need to check if permission is set as expected. Because of the influence of settings such as umask, we need to check whether the permissions are set as expected. If not, we should give them the correct permissions to ensure subsequent normal operation.

[skysiders]

Hi @bipinprasad .Could you take a look at this?Thanks

[bipinprasad]

Is there a check to see if the directory already exists with more generous permission than requested?

[skysiders]

Is there a check to see if the directory already exists with more generous permission than requested?

Hi @bipinprasad ,thanks for your review. I added a permission check to compare the file's current permissions with the granted permissions. These operations are performed after mkdirs. It can be seen that the mkdirs before the patch are all mkdirs(path, permission), and in the DistributedFileSystem, mkdirs(path, permission) has two functions. If the path does not exist, create the path and its parent directory, and respond to the The directory applies a permission, this permission is not permission(the second parameter), but (permission & umask). If path exists, permissions (permission & umask) will be applied to the folder. So the patch here just conforms to the program's mdirs for permission checking. So if the file exists and there are generous permission, mkdirs here will re-give the desired permissions to the folder.

[agresch]

It would be nice to move this functionality into some helper for common code - something lik e createDirWithPermissions(). But it's ok as is.

[skysiders]

It would be nice to move this functionality into some helper for common code - something lik e createDirWithPermissions(). But it's ok as is.

Hi @agresch ,Thanks for your review. I think it's better to check permission after set special permission.But not all permission is special such as 644 or 755. So if we move this function into some helper for common code perhaps make some misuse for others.