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

Implement Update method #134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement Update method #134

wants to merge 1 commit into from

Conversation

tbrisker
Copy link

@tbrisker tbrisker commented Apr 2, 2023

The update method allows atomically mutating a value in the map. If the key does not exist, the zero value for the value type is mutated and saved.

Fixes #133

Copy link

@ayushjain2809 ayushjain2809 left a comment

Choose a reason for hiding this comment

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

Thanks for the issue and PR. I have a similar use case.
I have a couple of suggestions, which combined will:

  • Pass exists to UpdateCb to allow distinguishing between zero-values and non-existent ones. This is also consistent with Upsert, RemoveCb
  • Allow UpdateCb to cancel the update if needed. Update would return the final value and a bool to indicate whether the update went through.

// It is called while lock is held, therefore it MUST NOT
// try to access other keys in same map, as it can lead to deadlock since
// Go sync.RWLock is not reentrant
type UpdateCb[V any] func(valueInMap V) V

Choose a reason for hiding this comment

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

Suggested change
type UpdateCb[V any] func(valueInMap V) V
type UpdateCb[V any] func(exists bool, valueInMap V) (newVal V, ok bool)


// Update an existing element using UpdateCb, assuming the key exists.
// If it does not, the zero value for the value type is passed to the callback.
func (m ConcurrentMap[K, V]) Update(key K, cb UpdateCb[V]) (res V) {

Choose a reason for hiding this comment

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

Suggested change
func (m ConcurrentMap[K, V]) Update(key K, cb UpdateCb[V]) (res V) {
func (m ConcurrentMap[K, V]) Update(key K, cb UpdateCb[V]) (res V, ok bool) {

The update method allows atomically mutating a value in the map.
If the key does not exist, the zero value for the value type is mutated
and saved.
@tbrisker
Copy link
Author

Thanks for the suggestion @ayushjain2809 , updated accordingly.

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

Successfully merging this pull request may close these issues.

Create an Update method for atomic Get+Set
2 participants