Change delivery failure tracking to work with hostnames instead of URLs #13437
Conversation
Gargron
force-pushed
the
fix-unavailable-inboxes-hosts
branch
from
5072de4
to
e6ad835
Compare
app/lib/delivery_failure_tracker.rb
Outdated
| end | ||
|
|
||
| def days | ||
| Redis.current.scard(exhausted_deliveries_key) || 0 | ||
| end | ||
|
|
||
| def available? | ||
| !UnavailableDomain.where(domain: @host).any? |
There was a problem hiding this comment.
maybe
!UnavailableDomain.exists?(domain: @host)| arr.reject(&method(:unavailable?)) | ||
| end | ||
| def without_unavailable(urls) | ||
| unavailable_domains_map = Rails.cache.fetch('unavailable_domains') { UnavailableDomain.pluck(:domain).each_with_object({}) { |domain, hash| hash[domain] = true } } |
There was a problem hiding this comment.
The number of unavailable domains could grow quite a bit, I'm not sure loading them all is safe…
Maybe caching a bloom filter instead, and then querying the database for domains that are possibly unavailable only would make more sense? Although rebuilding the bloom filter each time a domain is made available again would be costly…
There was a problem hiding this comment.
You're not wrong but at the same time the number of distinct hostnames will be smaller than the current number of distinct inbox URLs. I think bloom filters are a nice idea but maybe something one could do down the line when this becomes practical bottleneck.
There was a problem hiding this comment.
That is true, but although I'm not sure how Redis handles large sets, I'd think it handles it better than just creating a big Ruby hash for it… honestly I'm not sure what's best here, but loading the whole thing really doesn't sound right.
Gargron
force-pushed
the
fix-unavailable-inboxes-hosts
branch
from
86b9376
to
2b6b9fa
Compare
Gargron
force-pushed
the
fix-unavailable-inboxes-hosts
branch
from
2b6b9fa
to
71e534f
Compare
This should make reasoning about the behaviour of delivery failure tracking easier. It also ensures unavailable domains are not reset if Redis data is lost. By caching them through the Rails caching layer (which can potentially use a different Redis instance if configured correctly), it also helps memory usage, since a Redis instance configured for cache only can have a memory eviction policy.