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

Proposal to Simplify Get API by Returning a Pointer and Automating Resource Management #4294

Open
goupdate opened this issue Jan 27, 2025 · 1 comment

Comments

@goupdate
Copy link

goupdate commented Jan 27, 2025

Subject: Proposal to Simplify Get API by Returning a Pointer and Automating Resource Management

Dear Pebble Developers,

I would like to propose an improvement to the Get API to simplify its usage and prevent resource leaks.

Current Issue:

The Get method currently returns a value, a closer, and an error, which requires the user to explicitly call closer.Close(). This complicates the code and can lead to resource leaks if the developer forgets to call Close.

Proposal:

Modify the Get method to return a pointer to an object that automatically manages its resources. For example:

v, err := db.Get(key)
if err != nil {
    return err
}
fmt.Printf("%s %s\n", key, *v)

Benefits:

  1. Simplified API: Users no longer need to worry about calling Close.
  2. Prevent Resource Leaks: Resources are automatically released when they go out of scope.
  3. Consistency with Other Libraries: Many modern libraries handle resource management automatically, aligning with developer expectations.

Implementation:

This can be achieved using runtime.SetFinalizer or wrapper types that automatically close resources. For example:

type Value struct {
    data   []byte
    closer io.Closer
}

func (db *DB) Get(key []byte) (*Value, error) {
    data, closer, err := db.getInternal(key)
    if err != nil {
        return nil, err
    }
    value := &Value{data: data, closer: closer}
    runtime.SetFinalizer(value, func(v *Value) {
        v.closer.Close()
    })
    return value, nil
}

Alternatively, a wrapper type can be used:

type AutoCloseValue struct {
    data   []byte
    closer io.Closer
}

func (v *AutoCloseValue) Close() {
    v.closer.Close()
}

func (db *DB) Get(key []byte) (*AutoCloseValue, error) {
    data, closer, err := db.getInternal(key)
    if err != nil {
        return nil, err
    }
    return &AutoCloseValue{data: data, closer: closer}, nil
}

Why This Matters:

  • Ease of Use: Developers can focus on using the value without worrying about resource cleanup.
  • Reduced Errors: Forgetting to call Close is a common source of bugs, and this change eliminates that risk.
  • Modern Best Practices: Many libraries (e.g., database/sql in Go) already handle resource management internally, making this a familiar pattern for developers.

Conclusion:

This change would make the Pebble API more intuitive, reliable, and consistent with modern Go libraries. I believe it would greatly improve the developer experience and reduce the likelihood of resource leaks.

Jira issue: PEBBLE-334

@jbowens
Copy link
Collaborator

jbowens commented Jan 28, 2025

Finalizers have significant CPU overhead. They also may not run until well after a value becomes dead. This would lead to significant memory wastage within the block cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants