xinglin commented on PR #4869: URL: https://github.com/apache/hadoop/pull/4869#issuecomment-1244695714
Hi @steveloughran, Thanks for your comments and sharing your WIP. I updated the description of this PR. Please take a look and hopefully it provides more context. We have actually extended using an existing extention point in HADOOP-18144 , which is the getTrashRoot() implemementation in ViewFileSystem. It can now returns a viewfs trash root (a viewfs path) with the new localized trash root flag. That provides a fs specific trash root. I don't think a new ViewFSTrashPolicy is strictly required. ViewFileSystem is also different from other fs, such as hdfs/abfs in that ViewFileSystem is an indirection layer which points to other fs. It is possible that given the current moveToAppropriateTrasu() implementation which resolves the targetFs and use targetFs moveToTrash policy, the new viewFsTrashPolicy you suggest may also be bypassed. > interesting. In #4729 i'm proposing the ability to choose a trash policy for different fs fschemas, so the one for viewfs could be different to hdfs and then from abfs and s3a. The changes are handling the actual rename/delete checkpoint stuff, but not the changes you are proposing into Trash. > > having Trash choose its policy based on instanceof values is a bad path to follow. > > I'd be happier if the decision was done in a viewfs specific trash policy. Would there be a way to add the extra methods needed for the policy itself -the existing plugin point- to handle the viewfs details? > > That could maybe also line up with having the stores use the same extension points if they want to be extra clever. Not sure what they would want to do; the work I'm adding is to address these problems > > * ability to turn off trash on versioned s3a (no attempt to be adaptive) > * gcs & abfs: some extra resilience, stats collection if FS does it, and the option to have a moveToTrash() operation include an -expunge of old checkpoints. That's to stop the stores collecting too much old data that's never cleaned up. > > I'm away for a week, back sept 19. if you looked at my PR and especially the schema-specific plugin, would that work if you added the trash root awareness to the policy? if so, you could take that bit of the PR and run with it, as you are ahead of me and my work. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
