Skip to content

Аdd a metric about the maximum number of collisions in lrushah #826

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

Merged
merged 1 commit into from
Jan 13, 2023
Merged

Аdd a metric about the maximum number of collisions in lrushah #826

merged 1 commit into from
Jan 13, 2023

Conversation

sakateka
Copy link
Contributor

In continuation of the fix added here 90831af
I would like to suggest adding a metric about the maximum number of collisions that happened in lruhash.
This will be the best indicator of a similar problem in the future.

Copy link
Member

@wcawijngaards wcawijngaards left a comment

Choose a reason for hiding this comment

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

The change looks good. This code can be added.

@wcawijngaards wcawijngaards merged commit 785c938 into NLnetLabs:master Jan 13, 2023
@sakateka sakateka deleted the lruhash_max_collisions branch January 13, 2023 10:00
wcawijngaards added a commit that referenced this pull request Jan 13, 2023
- Merge #826: Аdd a metric about the maximum number of collisions in
  lrushah.
@wcawijngaards
Copy link
Member

Thanks for the additional performance counter! Perhaps it can prevent other issues like this.

I have merged it in to the code repository. Also documentation is added, for the statistics entries, do you like the text for it; it can be seen in the commit 469133e ?

@sakateka
Copy link
Contributor Author

It looks very good, thank you!
I accidentally forgot about the documentation).

Perhaps it was worth writing very large (hundreds)
Because several hundred in one bin already affect performance.
In practice, during load testing, I have not seen a value greater than 20.

@wcawijngaards
Copy link
Member

Okay, improved the wording to add that detail! Useful to know what amount is trouble.

jedisct1 added a commit to jedisct1/unbound that referenced this pull request Jan 15, 2023
* nlnet/master:
  - Improve documentation for NLnetLabs#826, describe the large collisions amount.
  Changelog note and documentation for NLnetLabs#826 - Merge NLnetLabs#826: Аdd a metric about the maximum number of collisions in   lrushah.
  add a metric about the maximum number of collisions in lrushah
  Code repository continues with version 1.17.2.
  - Fix python version detection in configure.
  - Fix python module install path detection.
  Changelog note for 1.17.1rc2 fix. - Fix wildcard in hyperlocal zone service degradation, reported   by Sergey Kacheev. This fix is included in 1.17.1rc2.
  - Fix wildcard in hyperlocal zone service degradation, reported   by Sergey Kacheev.
  - Fix NLnetLabs#823: Response change to NODATA for some ANY queries since   1.12, tested on 1.16.1.
  Changelog note for tag for 1.17.1rc1. - Tag for 1.17.1 release.
  Add Mastodon link
  Add Mastodon
  - Update github workflows to use checkout v3.
  - Fix windows compile for libunbound subprocess reap comm point closes.
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.

2 participants