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

change RLock to Lock for MutableTree.VersionExists #448

Merged
merged 2 commits into from
Dec 1, 2021

Conversation

yun-yeo
Copy link
Contributor

@yun-yeo yun-yeo commented Dec 1, 2021

We monitored, sometimes we met concurrent read write map access panic and found there is some miss-configured lock.

MutableTree.VersionExists has write operation, so we have to change to use write lock.

iavl/mutable_tree.go

Lines 64 to 79 in 577c9a8

func (tree *MutableTree) VersionExists(version int64) bool {
tree.mtx.RLock()
defer tree.mtx.RUnlock()
if tree.allRootLoaded {
return tree.versions[version]
}
has, ok := tree.versions[version]
if ok {
return has
}
has, _ = tree.ndb.HasRoot(version)
tree.versions[version] = has
return has
}

Copy link
Member

@tac0turtle tac0turtle 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 PR!!

@tac0turtle tac0turtle merged commit 8037996 into cosmos:master Dec 1, 2021
@robert-zaremba
Copy link
Collaborator

@odeke-em it turns out that the Mutex improvements are mis-targetted. I'm really curious why we can't use RWMutex. Do you have an idea?

@tac0turtle
Copy link
Member

they can be used and are used in places, just can't be used here

@yun-yeo yun-yeo deleted the bugfix/mutable-tree-mutex branch December 6, 2021 01:17
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.

3 participants