Opened 5 years ago

Closed 5 years ago

#11683 closed enhancement (implemented)

Stop caching extra-info documents on directory mirrors

Reported by: karsten Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I suggest stopping to cache extra-info documents on directory mirrors.

Most clients don't need extra-info documents for their normal operation. The only use case that comes to mind is external tools configuring a local tor client to provide them with extra-info documents. Note that we wouldn't break these tools, because they could still fetch from the directory authorities. Though I assume there are very few requests for extra-info documents in the network.

There are also not many directory mirrors in the network that serve extra-info documents: in the first weeks of April 2014, only 0.46% of server descriptors contained the "caches-extra-info" line, so 0.46% * 5,000 = 23 relays. Note that 9 of these are directory authorities and only 14 are directory mirrors.

But still, these 14 directory mirrors need to keep their cache up-to-date by fetching extra-info documents from the directory authorities. If there are fewer than 14 clients downloading extra-info documents from the caches, the caches put even more load on directory authorities just to keep their caches recent.

All in all, I'd say let's get rid of extra-info documents on directory mirrors. I can write a patch, unless somebody else enjoys removing (mostly) unused code as much as I do.

Child Tickets

Change History (7)

comment:1 Changed 5 years ago by nickm

Would this remove much code? It seems that the code to fetch and store extrainfo documents already needs to be there (so clients can fetch them), and the code to serve them already needs to be there (so authorities can serve them). What would this actually remove, other than the ability to set the caches-extra-info flag?

comment:2 Changed 5 years ago by karsten

You're right, this wouldn't remove as much code as I had expected. But it's going to remove the need for directory mirrors to keep their extra-info caches recent. And it's going to make the spec slightly simpler.

So, while going through the code, I found that clients currently rely on "caches-extra-info" when selecting a directory to fetch extra-info documents from. I believe that's a bug. Clients should always expect the directory authorities to support extra infos, regardless of whether they contain that line in their descriptor or not. (One might argue whether clients should prefer non-authorities over authorities for downloads, but believing that authorities don't serve extra-infos is just not correct.)

How about we add a patch for that to 0.2.5 and postpone possibly taking away the "caches-extra-info" line from directory mirrors until 0.2.6 and from directory authorities until 0.2.7?

Please find my branch bug11683.

(Unrelated to this, I'm not 100% certain whether the use of is_trusted is correct in that function. The bridge authority is trusted, too, but we wouldn't want to ask it for microdescriptors, would we?)

comment:3 Changed 5 years ago by karsten

Status: newneeds_review

comment:4 Changed 5 years ago by nickm

Maybe it would make more sense to have authorities, and nobody else, include "caches-extra-info" in their descriptors? That way existing clients won't break.

comment:5 Changed 5 years ago by karsten

You mean something like this?

diff --git a/src/or/router.c b/src/or/router.c
index 86cefc9..5ecf065 100755
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -2370,7 +2370,7 @@ router_dump_router_to_string(routerinfo_t *router,
     has_extra_info_digest ? "extra-info-digest " : "",
     has_extra_info_digest ? extra_info_digest : "",
     has_extra_info_digest ? "\n" : "",
-    options->DownloadExtraInfo ? "caches-extra-info\n" : "",
+    options->V3AuthoritativeDir ? "caches-extra-info\n" : "",
     onion_pkey, identity_pkey,
     family_line,
     we_are_hibernating() ? "hibernating 1\n" : "",

(I can turn this into a real commit with changes file if this is the patch you want to apply.)

This patch is certainly simpler. But it has the downside that directory authorities will have to include "caches-extra-info" forever, or at least until we teach clients not to rely on that line plus two or three release cycles. (That's why I suggested 0.2.5 above, to save a release cycle.)

If either patch is too hot for 0.2.5, how about we defer until 0.2.6? Nothing urgent here.

comment:6 Changed 5 years ago by nickm

Milestone: Tor: 0.2.???Tor: 0.2.6.x-final

Sure. (It was already in 0.2.???)

comment:7 Changed 5 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Okay, I applied all of this except for the part where we make "DownloadExtraInfo" no longer set caches-extra-info. As far as I can tell, that wouldn't actually save any bandwidth: the nodes in question with DownloadExtraInfo set would still fetch the ExtraInfo document -- they just wouldn't share 'em.

Note: See TracTickets for help on using tickets.