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

Acquire read lock instead of exclusive lock for langBaseCache. #4279

Merged
merged 3 commits into from
Nov 19, 2019

Conversation

arijitAD
Copy link

@arijitAD arijitAD commented Nov 15, 2019

Reduces lock contention in langBaseCache by using RWMutex instead of Mutex.


This change is Reviewable

@arijitAD arijitAD requested review from manishrjain and a team as code owners November 15, 2019 13:40
@arijitAD arijitAD requested review from poonai and removed request for a team November 15, 2019 13:41
Copy link
Contributor

@poonai poonai left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @arijitAD, @balajijinnah, and @manishrjain)


tok/langbase.go, line 55 at r1 (raw file):

	langBaseCache.Lock()
	defer langBaseCache.Unlock()

After aquiring lock check once again whether. cache contains the lang or not. Some other routine may update the value

Copy link
Author

@arijitAD arijitAD left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @balajijinnah and @manishrjain)


tok/langbase.go, line 55 at r1 (raw file):

Previously, balajijinnah (balaji) wrote…

After aquiring lock check once again whether. cache contains the lang or not. Some other routine may update the value

Done.

Copy link
Contributor

@poonai poonai left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @manishrjain)

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @manishrjain)

@arijitAD arijitAD merged commit e8cfd0d into master Nov 19, 2019
@arijitAD arijitAD deleted the arijitAD/rwlock_langbase branch November 19, 2019 16:29
@devinbost
Copy link

@arijitAD Could this PR have impacted upsert performance?

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

Successfully merging this pull request may close these issues.

4 participants