Removed force_close from all downloaders #4453
Conversation
This flag causes large syncs to fail under certain circumstances, because the port range is used up too fast and reusing ports in time_wait is not appreciated by servers. We cannot possibly fix all broken http servers in the wild. fixes pulp#4452
There was a problem hiding this comment.
We should try this and see if there are regressions reported by users. It's hard to know what will happen because this fix was introduced to workaround real webservers in the wild, so our automated tests won't help us. Still though, let's try it.
Also FYI, the original issue we worked around was a checksum mismatch bug, so if we start getting those reported we should know this removal could be related.
@mdellweg ty for making this.
|
@dralley I think you're an expert in our downloaders, wdyt about being second reviewer? |
There was a problem hiding this comment.
I'm also +1 on this, my only question (and I'm not strongly in favor of requiring this) is whether we ought to stick it behind an off-by-default feature flag just in case.
But either way we could undo the change fairly easily and quickly if we start encountering such issues again. As long as we recognize them as such.
Note that since we generally call remote.get_downloader() I believe we still end up with a new downloader on every request anyway? And, thus, probably a new session? https://github.com/pulp/pulpcore/blob/main/pulpcore/app/models/repository.py#L507
|
We'll add the feature flag if it is still encountered as an issue. |
|
Backporting as requested in AAH-2857 |
Backport to 3.18: 💚 backport PR created✅ Backport PR branch: Backported as #4794 🤖 @patchback |
Backport to 3.21: 💚 backport PR created✅ Backport PR branch: Backported as #4793 🤖 @patchback |
Backport to 3.22: 💚 backport PR created✅ Backport PR branch: Backported as #4796 🤖 @patchback |
Backport to 3.23: 💚 backport PR created✅ Backport PR branch: Backported as #4798 🤖 @patchback |
Backport to 3.39: 💚 backport PR created✅ Backport PR branch: Backported as #4797 🤖 @patchback |
Backport to 3.41: 💚 backport PR created✅ Backport PR branch: Backported as #4795 🤖 @patchback |
This flag causes large syncs to fail under certain circumstances, because the port range is used up too fast and reusing ports in time_wait is not appreciated by servers.
We cannot possibly fix all broken http servers in the wild.
fixes #4452