Calculate ipset maxelem instead of hardcoding #20
Conversation
python/geoipsets/maxmind.py
Outdated
| @@ -160,7 +168,9 @@ def build_sets(self, country_code_map: dict, zip_ref: ZipFile, dir_prefix: str, | |||
| ipset_file = ipset_dir / set_name | |||
| if not ipset_file.is_file(): | |||
| with open(ipset_file, 'a') as f: | |||
| f.write("create " + set_name + " hash:net " + inet_family + " maxelem 131072 comment\n") | |||
| # round up to the next next power of 2 for a load ratio < 0.5 | |||
There was a problem hiding this comment.
I'm not sure maxelem has any bearing on the actual size of the underlying hash data structure used by ipset -- and therefore any effect on the load ratio of the hash table. It is simply a cap on the number of elements in a set.
There is a separate hashsize setting that controls the initial size of the hash table, but the actual size is managed internally by ipset based on the number of set elements at any given time. (According to the manpage the size is doubled if existing size is exhausted but there is probably more to it than that.)
Given this, I think setting maxelem to the next power of 2 (instead of the next next power of 2) is sufficient. If you agree, please push that change to the PR. If I've missed something, though, please do let me know.
There was a problem hiding this comment.
You are absolutely right. Now begging the question: Do we set the exact maxelem since it doesn't need to be a power of 2
according to the man page?
There was a problem hiding this comment.
Let's stick with a power of 2. The default (65536) and example (2048) for maxelem in the manpage are both a power of 2 and doing otherwise just feels wrong.
|
Hi @johnmave126 - thanks for this enhancement. I've added one note to the PR and will merge once that's resolved. There are a few other fixes and enhancements on the way, so I will wait for those to accumulate before cutting a new release that includes this PR. |
The Maxmind US.ipv6 set contains more than 131072 entries as of now which will raise errors when trying to import. This PR counts the number of entry first and rounds up to the next next power of 2 to avoid allocating an ipset that is too small.