-
Notifications
You must be signed in to change notification settings - Fork 207
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
base: feature/filter
Are you sure you want to change the base?
Conversation
.gitignore
Outdated
@@ -21,3 +21,4 @@ lint.log | |||
azure-storage-fuse | |||
bfusemon | |||
test/scripts/dirIterate.go | |||
filterbin |
There was a problem hiding this comment.
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.
component/azstorage/config.go
Outdated
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"` |
There was a problem hiding this comment.
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.
component/azstorage/config.go
Outdated
|
||
az.stConfig.blobFilter = opt.BlobFilter | ||
if len(opt.BlobFilter) > 0 { | ||
fmt.Println(opt.BlobFilter) |
There was a problem hiding this comment.
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,
component/azstorage/config.go
Outdated
if len(opt.BlobFilter) > 0 { | ||
fmt.Println(opt.BlobFilter) | ||
erro := filter.ParseInp(&opt.BlobFilter) | ||
fmt.Println(len(filter.GlbFilterArr)) |
There was a problem hiding this comment.
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
component/azstorage/azstorage.go
Outdated
@@ -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 |
There was a problem hiding this comment.
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.
component/azstorage/config.go
Outdated
return erro | ||
} | ||
} | ||
//blobfilter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deadcode can be removed.
component/azstorage/azstorage.go
Outdated
return resp, err | ||
} | ||
|
||
if len(az.stConfig.blobFilter) > 0 { |
There was a problem hiding this comment.
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.
component/azstorage/azstorage.go
Outdated
|
||
if len(az.stConfig.blobFilter) > 0 { | ||
fv1 := &filter.FileValidator{ | ||
FilterArr: filter.GlbFilterArr, |
There was a problem hiding this comment.
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.
component/azstorage/azstorage.go
Outdated
@@ -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) |
There was a problem hiding this comment.
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.
internal/filter/filter.go
Outdated
"github.com/Azure/azure-storage-fuse/v2/internal" | ||
) | ||
|
||
var GlbFilterArr [][]Filter //it will store the fliters, outer array splitted by ||, inner array splitted by && |
There was a problem hiding this comment.
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.
internal/filter/filter.go
Outdated
|
||
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 |
There was a problem hiding this comment.
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,
tools/filter/formatfilter.go
Outdated
@@ -0,0 +1,34 @@ | |||
package main |
There was a problem hiding this comment.
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
…ed to filter module
component/azstorage/azstorage.go
Outdated
//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 |
There was a problem hiding this comment.
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.
internal/filter/accesstier.go
Outdated
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)) { |
There was a problem hiding this comment.
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.
internal/filter/accesstier.go
Outdated
|
||
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 "=") |
There was a problem hiding this comment.
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")
@t-yashmahale please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
1 similar comment
@t-yashmahale please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
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.