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

Update schema_registry.go #147

Closed
wants to merge 3 commits into from
Closed

Update schema_registry.go #147

wants to merge 3 commits into from

Conversation

oscar067
Copy link

@oscar067 oscar067 commented Aug 5, 2022

When calling to retrieve the schemaInfo the library github.com/riferrei/srclient v0.5.4 calls this code which calls to Rlock, creating a performance problem in the library, the following code is a workaround to avoid calling to that cache always

// RLock locks rw for reading.
//
// It should not be used for recursive read locking; a blocked Lock
// call excludes new readers from acquiring the lock. See the
// documentation on the RWMutex type

func (client *SchemaRegistryClient) getVersion(subject string, version string) (*Schema, error) {

	if client.getCachingEnabled() {
		cacheKey := cacheKey(subject, version)
		client.subjectSchemaCacheLock.RLock()
		cachedResult := client.subjectSchemaCache[cacheKey]
		client.subjectSchemaCacheLock.RUnlock()
		if cachedResult != nil {
			return cachedResult, nil
		}
	}

schema_registry.go Outdated Show resolved Hide resolved
schema_registry.go Outdated Show resolved Hide resolved
oscar0672 and others added 2 commits August 5, 2022 15:16
Co-authored-by: Mostafa Moradian <mostafamoradian0@gmail.com>
Co-authored-by: Mostafa Moradian <mostafamoradian0@gmail.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@oscar067
Copy link
Author

oscar067 commented Aug 5, 2022

reviewed and accepted changes!

@mostafa
Copy link
Owner

mostafa commented Aug 10, 2022

@oscar067
After merging #149, I found conflicts here. I fetched the changes to solve the conflicts, but there are too many changes. So, I created #151 to address the issue while considering what @riferrei suggested.

@mostafa mostafa linked an issue Aug 11, 2022 that may be closed by this pull request
@mostafa
Copy link
Owner

mostafa commented Aug 12, 2022

I'll close this in favor of #151.

@mostafa mostafa closed this Aug 12, 2022
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.

Problem with schema registry cache- Workaround
3 participants