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

Need for synchronization while looking up Zones #305

Open
srijeet0406 opened this issue Nov 17, 2023 · 4 comments · May be fixed by #306
Open

Need for synchronization while looking up Zones #305

srijeet0406 opened this issue Nov 17, 2023 · 4 comments · May be fixed by #306

Comments

@srijeet0406
Copy link

I was wondering what the point of synchronizing methods like allRRsets, oneRRSet, lookup and exactName is. Could we possibly use a ReEntrantReadWriter Lock (only Read locks in this regard) to attain the same behavior. This should result in considerable performance increase and less resource contention. Any feedback is welcome.

@ibauersachs
Copy link
Member

This might be possible. The Zone code is rather old and I wonder if might be better to rewrite some parts, especially the ones that use Object to operate on either a single RRSet or on a list of RRSets.

In what case do you run into lock contentions? Multiple parallel reads?

@srijeet0406
Copy link
Author

@ibauersachs Yes, that Object comparison on RRSet or a list oif RRSets was another one of the areas that I was looking at. But in this case, yes, I'm seeing bottlenecks and resource contentions on the findRecords method (which calls the lookup method) during multiple reads. I can create a PR for the same if you think it would be helpful.

@ibauersachs
Copy link
Member

Thanks, a PR is always welcome!
I'm not sure how well the Zone class is test covered, please bear this in mind when refactoring things.

@srijeet0406 srijeet0406 linked a pull request Nov 30, 2023 that will close this issue
@srijeet0406
Copy link
Author

@ibauersachs PR is out for review.

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 a pull request may close this issue.

2 participants