fileservice: fix object storage semaphore

[reusee]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #17001 #17793

What this PR does / why we need it:

fix object storage semaphore

---

PR Type

Bug fix, Enhancement

---

Description

• Added a new readerFunc type that implements the io.Reader interface to improve code modularity.
• Enhanced the objectStorageSemaphore to safely release resources using sync.OnceFunc, ensuring release on errors and closure.
• Increased the default concurrency limit for S3FS from 100 to 1024, allowing more concurrent operations.

---

Changes walkthrough 📝

	Relevant files
Enhancement	io.go Add `readerFunc` type implementing `io.Reader`                      pkg/fileservice/io.go Added a new type readerFunc implementing io.Reader. Implemented the Read method for readerFunc. +8/-0      s3_fs.go Increase default concurrency limit for S3FS                            pkg/fileservice/s3_fs.go Increased default concurrency limit from 100 to 1024. +1/-1
Bug fix	object_storage_semaphore.go Improve resource release in object storage semaphore          pkg/fileservice/object_storage_semaphore.go Introduced sync.OnceFunc for safe release of resources. Modified Read method to ensure release on error. Updated readCloser to use readerFunc. +16/-6

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Resource Management The new implementation uses sync.OnceFunc for resource release, which is good. However, it's important to ensure that the release function is always called, even in edge cases. Performance Impact The default concurrency limit for S3FS has been increased from 100 to 1024. This significant increase might impact system resources and should be thoroughly tested.

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Add context cancellation check to improve responsiveness and prevent unnecessary operations Consider adding a context check before performing the read operation to ensure that the context hasn't been canceled. This can help prevent unnecessary operations and improve responsiveness to cancellation requests. pkg/fileservice/object_storage_semaphore.go [80-87] r: readerFunc(func(buf []byte) (n int, err error) { - n, err = r.Read(buf) - if err != nil { - // release if error + select { + case <-ctx.Done(): release() + return 0, ctx.Err() + default: + n, err = r.Read(buf) + if err != nil { + release() + } + return } - return }), Apply this suggestion Suggestion importance[1-10]: 9 Why: Adding a context cancellation check is a significant improvement that enhances responsiveness and prevents unnecessary operations, addressing potential issues with context management.	9
Best practice	Define default values as named constants to improve code maintainability Consider making the default concurrency value a named constant at the package level, rather than hardcoding it directly in the function. This improves maintainability and allows for easier changes in the future. pkg/fileservice/s3_fs.go [115-117] +const defaultConcurrency = 1024 + if concurrency == 0 { - concurrency = 1024 + concurrency = defaultConcurrency } Apply this suggestion Suggestion importance[1-10]: 8 Why: Defining default values as named constants enhances maintainability and readability, making future changes easier and reducing the risk of errors from hardcoding values.	8
Maintainability	Improve readability by using a named function for the wrapped read operation Consider using a more descriptive name for the readerFunc closure, such as wrappedRead, to better convey its purpose of wrapping the original reader with error handling and semaphore release logic. pkg/fileservice/object_storage_semaphore.go [80-87] r: readerFunc(func(buf []byte) (n int, err error) { - n, err = r.Read(buf) - if err != nil { - // release if error - release() + wrappedRead := func(buf []byte) (int, error) { + n, err := r.Read(buf) + if err != nil { + release() + } + return n, err } - return + return wrappedRead(buf) }), Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion improves code readability by using a more descriptive name for the closure, which helps convey its purpose. However, it is not crucial for functionality.	6