Eliminate rsync-strategy #80
Milestone
Comments
ydahhrk
added a commit
that referenced
this issue
Jun 30, 2023
Only STRICT synchronizations are allowed now. This orphans --rsync.arguments-recursive, so it has been deprecated and no-op'd as well. Fixes #80.
ydahhrk
added a commit
that referenced
this issue
Sep 9, 2023
It's a bit smarter now. Addresses a bunch of issues at once, though it still needs several tweaks and testing: - #78: Provide a dedicated namespace for each RRDP notification, to prevent malicious RPPs from overriding files from other RPPs. - #79: RRDP session and serial are no longer cached in RAM; they're extracted from cached notification files as they are needed. This prevents all RRDP from being considered outdated during startup. - #80: rsync-strategy has been removed. - #81: The cache now retains RRDP files. The refactor has been more intrusive than intended. I've been retouching the core loop and rrdp/https code, which has yielded the following further disinfections: - #77: Refactor the HTTP code so 304 is handled as success, despite no file modifications having been made. - It seems the old code was refusing to download RPPs via RRDP when said RPP wasn't also (unrelatedly) served via rsync. This seemed to stem from an old RFC misunderstanding from the previous developer. - I've deprecated `rsync.priority` and `rrdp.priority`, mostly just to simplify the code. I haven't seen anyone using these config fields, and I think SIAs and/or randomness should be the ones to decide which protocol is preferred for a given RPP, not Fort's admin. - However, I have also decided to deprecate `shuffle_tal_uris`, because I also suspect it's completely unused, and would like to hear some complaints otherwise. - Deprecated `rsync.arguments-flat`, because non-recursive rsyncs are not needed anymore. - Since RRDP files are no longer deleted immediately after use, the `DEBUG_RRDP` compilation has lost its purpose, so I deleted it. - The code was using `HASH_ADD_STR` on strings contained outside of the node structure. This is illegal according to uthash's documentation, and might have induced some crashes in the past.
ydahhrk
added a commit
that referenced
this issue
Nov 21, 2023
rsync-ing every RPP separately is prohibitely expensive when RRDP is unavailable. As it turns out, rsync-ing the repository root happens to be an unwritten standard. Fort was far from the only one doing it, and people expect it. This means I will eventually have to come up with a different way to box RRDP RPPs, as the current implementation induces too many redownloads when rsync needs to be fall-backed into. I will have to leave that outside of 1.6.0 however, as I've fixed too much stuff already, and I need a new release urgently. Sort of reverts #80, though the flag will remain deleted. I don't think there's a point in offloading this decision to the user.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The
rsync-strategyoptimization assumes most of the repository is accessed through rsync. Now that RRDP is the dominant repository download protocol, it's probably just causing lots of redundant file downloads.Either way, this will need to be inevitably fixed as part of #74.
Branch rrdp-refactor has a WIP on this bug.
The text was updated successfully, but these errors were encountered: