Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flexible File Filtering Feature for Blobfuse Mounting #1435

Merged
merged 32 commits into from
Dec 9, 2024

Conversation

t-yashmahale
Copy link

This pull request introduces advanced filtering capabilities to Blobfuse, empowering users to selectively mount cloud files based on size, regex patterns, modification time, and formats like PDF, JPG, TXT, and more. Efficient thread pool management at both file and filter levels ensures swift processing, enhancing user flexibility and optimizing resource utilization. This enhancement streamlines the file mounting process, enabling users to tailor their cloud storage access with precision and performance.

.gitignore Outdated
@@ -21,3 +21,4 @@ lint.log
azure-storage-fuse
bfusemon
test/scripts/dirIterate.go
filterbin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is this binary file _debug* which can be removed from the PR.

CPKEnabled bool `config:"cpk-enabled" yaml:"cpk-enabled"`
CPKEncryptionKey string `config:"cpk-encryption-key" yaml:"cpk-encryption-key"`
CPKEncryptionKeySha256 string `config:"cpk-encryption-key-sha256" yaml:"cpk-encryption-key-sha256"`
BlobFilter string `config:"blobFilter" yaml:"blobFilter"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this new config towards the end of the structure so that diff does not show existing items.


az.stConfig.blobFilter = opt.BlobFilter
if len(opt.BlobFilter) > 0 {
fmt.Println(opt.BlobFilter)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can move fmt.Println to log.Debug as blobfuse will have no console attached to print,

fmt.Println(len(filter.GlbFilterArr))
fmt.Println(len(opt.BlobFilter))
if erro != nil {
log.Err("no filters applied, mount failed")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For log.Debug/Err we are following a format which is consistent across all our logs. Just check other existing logs in this file and modify accordingly.

if len(opt.BlobFilter) > 0 {
fmt.Println(opt.BlobFilter)
erro := filter.ParseInp(&opt.BlobFilter)
fmt.Println(len(filter.GlbFilterArr))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These shall be converted to log.Debug or log.Info

@@ -331,6 +333,10 @@ func (az *AzStorage) StreamDir(options internal.StreamDirOptions) ([]*internal.O
// increment streamdir call count
azStatsCollector.UpdateStats(stats_manager.Increment, streamDir, (int64)(1))

//check for filters provided
if len(az.stConfig.blobFilter) > 0 { //only apply filter if user has given
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After parsing we might have created a filter object so instead of checking the string length here rather we can check if that object is nil or not, will be a faster check.

return erro
}
}
//blobfilter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deadcode can be removed.

return resp, err
}

if len(az.stConfig.blobFilter) > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking length try to check object is nil or not.


if len(az.stConfig.blobFilter) > 0 {
fv1 := &filter.FileValidator{
FilterArr: filter.GlbFilterArr,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we can name the variables better than fv1 and GlbFilterArr.

@@ -331,6 +333,10 @@ func (az *AzStorage) StreamDir(options internal.StreamDirOptions) ([]*internal.O
// increment streamdir call count
azStatsCollector.UpdateStats(stats_manager.Increment, streamDir, (int64)(1))

//check for filters provided
if len(az.stConfig.blobFilter) > 0 { //only apply filter if user has given
new_list = filter.ApplyFilterOnBlobs(new_list)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we keep the input and output variables same ? just think it might not be a safer way to do conversion.

"github.com/Azure/azure-storage-fuse/v2/internal"
)

var GlbFilterArr [][]Filter //it will store the fliters, outer array splitted by ||, inner array splitted by &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As part of design we shall move these filters to be a class so that there can be multiple instances of such filters across the blobfuse code. Current design of having it as a global variable restricts that only one such filter can exists in scope of blobfuse instance.


var GlbFilterArr [][]Filter //it will store the fliters, outer array splitted by ||, inner array splitted by &&

func ApplyFilterOnBlobs(fileInfos []*internal.ObjAttr) []*internal.ObjAttr { //function called from azstorage.go streamDir func
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From outside it shall be an object based call.

  • read config as a string from cli
  • parse it and create a filter object
  • filtering code shall be executed on this object
  • if filters are not applied then object shall be nil
  • if not nil then apply the filters and it will return back a new slice.
  • this model will allow us to have multiple instances of filter across the code if required,

@@ -0,0 +1,34 @@
package main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tools/filter can be removed

//check for filters provided
if az.stConfig.filters != nil { //only apply if user has given filter
filtered_list := az.stConfig.filters.ApplyFilterOnBlobs(new_list)
// return filtered_list, *new_marker, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to return here instead of copying over the slice again.

func (filter AccessTierFilter) Apply(fileInfo *internal.ObjAttr) bool {
// fmt.Println("AccessTier filter ", filter, " file name ", (*fileInfo).Name) DEBUG PRINT
fmt.Println("inside filter tier ", filter, " with given tier ", filter.tier, " and file tier ", fileInfo.Tier)
if (filter.opr == "=") && (filter.tier == strings.ToLower(fileInfo.Tier)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple string checks can be avoided.


func giveAccessTierFilterObj(singleFilter *string) (Filter, error) {
(*singleFilter) = strings.Map(StringConv, (*singleFilter)) //remove all spaces and make all upperCase to lowerCase
sinChk := (*singleFilter)[4:5] //single char after tier (ex- tier=hot , here sinChk will be "=")
Copy link
Member

@vibhansa-msft vibhansa-msft Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of magic numbers use len("keyword")

@vibhansa-msft vibhansa-msft added this to the v2-2.4.0 milestone Jul 30, 2024
@vibhansa-msft vibhansa-msft removed this from the v2-2.4.0 milestone Nov 4, 2024
@jainakanksha-msft jainakanksha-msft added this to the v2-2.4.1 milestone Nov 4, 2024
@vibhansa-msft vibhansa-msft merged commit 8fb1141 into Azure:feature/filter Dec 9, 2024
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants