Relax the rend cache failure cleanup timer
rend_cache_failure_clean() is called every second and the reason is that we want to make the client wait as little as possible so we try our best to cleanup the failure cache.
This is not ideal CPU wise since the cache could technically grow to the number of possible introduction point in the network thus making it a larger loop every second.
Let's relax the cleanup timer here to 5 minutes (expiry time of an entry) and at each lookup, if the entry did expire, clean it and return that "we do not have an entry". This will not address the size of the cache that can grows but that's fine since we can handle that in the OOM. Also, a cache that has the maximum number of entries is a valid use case.
Activity
changed milestone to %Tor: unspecified
This is relevant to prop224 client implementation as well.
Trac:
Keywords: N/A deleted, tor-client, tor-hs added
Type: defect to enhancement
Points: small/medium to 1
Parent: N/A to #17242 (moved)-
rend_cache_failure_clean(now); hs_cache_client_intro_state_clean(now);... is called every 30 seconds actually which is done by
rend_cache_failure_clean_callback().- Cleanup any entry that expired at lookup.
- v2 entries expire after 5 minutes. and v3 after 2 minutes.
So, calling the callback every 2 minutes could be what we want to fit both use case OR we bring down to 2 minutes the v2.
Trac:
Parent: #17242 (moved) to #23300 (moved) Trac:
Cc: N/A to neel@neelc.org
Owner: N/A to neel
Status: new to assignedMy PR is available here: https://github.com/torproject/tor/pull/305
-
Unfortunately, the patch is missing the "Cleanup expired entries on lookup for v3", only v2 in the branch has been modified.
You probably simply want to call
hs_cache_client_intro_state_clean()before callingcache_client_intro_state_lookup()... or something around those lines :).Trac:
Status: needs_review to needs_revision Replying to dgoulet:
You probably simply want to call
hs_cache_client_intro_state_clean()before callingcache_client_intro_state_lookup()... or something around those lines :).I tried this, and get errors like this:
hs_client/client_pick_intro: [forking] Sep 05 20:12:27.607 [err] tor_assertion_failed_: Bug: src/test/test_hs_client.c:431: test_client_pick_intro: Assertion !ip failed; aborting. (on Tor 0.3.5.0-alpha-dev e3d1c98f6add4d09) Sep 05 20:12:27.613 [err] Bug: Assertion !ip failed in test_client_pick_intro at src/test/test_hs_client.c:431. Stack trace: (on Tor 0.3.5.0-alpha-dev e3d1c98f6add4d09) Sep 05 20:12:27.613 [err] Bug: 0x165515c <log_backtrace_impl+0x4c> at /usr/home/neel/code/tor/tor/src/test/test (on Tor 0.3.5.0-alpha-dev e3d1c98f6add4d09) Sep 05 20:12:27.613 [err] Bug: 0x1651420 <tor_assertion_failed_+0xa0> at /usr/home/neel/code/tor/tor/src/test/test (on Tor 0.3.5.0-alpha-dev e3d1c98f6add4d09) Sep 05 20:12:27.613 [err] Bug: 0x1390475 <dir_common_construct_vote_3+0x57cb5> at /usr/home/neel/code/tor/tor/src/test/test (on Tor 0.3.5.0-alpha-dev e3d1c98f6add4d09) Sep 05 20:12:27.613 [err] Bug: 0x14ac573 <testcase_run_one+0x373> at /usr/home/neel/code/tor/tor/src/test/test (on Tor 0.3.5.0-alpha-dev e3d1c98f6add4d09) Sep 05 20:12:27.614 [err] Bug: 0x14ac9ff <tinytest_main+0x1af> at /usr/home/neel/code/tor/tor/src/test/test (on Tor 0.3.5.0-alpha-dev e3d1c98f6add4d09) Sep 05 20:12:27.614 [err] Bug: 0x14ab9f3 <main+0x2f3> at /usr/home/neel/code/tor/tor/src/test/test (on Tor 0.3.5.0-alpha-dev e3d1c98f6add4d09) [Lost connection!] [client_pick_intro FAILED]I am thinking about calling
hs_cache_client_intro_state_clean()incache_client_intro_state_lookup(). Is this okay?UPDATE Nevermind, this fails also, it's basically the same thing.
I got it passing tests provided I remove this code from
src/test/test_hs_client.c(functiontest_client_pick_intro():/* Now also mark the chosen one as failed: See that we can't get any intro points anymore. */ hs_cache_client_intro_state_note(&service_kp.pubkey, &chosen_intro_point->auth_key_cert->signed_key, INTRO_POINT_FAILURE_TIMEOUT); extend_info_t *ip = client_get_random_intro(&service_kp.pubkey); tor_assert(!ip);The reason why I think it's okay to remove this is because
hs_cache_client_intro_state_find()andhs_cache_client_intro_state_note()would be callinghs_cache_client_intro_state_clean()which does it (and this test fails from the code I mentioned above if I did not delete it).It is already committed.
-
Replying to neel:
I got it passing tests provided I remove this code from
src/test/test_hs_client.c(functiontest_client_pick_intro():{{{ /* Now also mark the chosen one as failed: See that we can't get any intro points anymore. */ hs_cache_client_intro_state_note(&service_kp.pubkey, &chosen_intro_point->auth_key_cert->signed_key, INTRO_POINT_FAILURE_TIMEOUT); extend_info_t *ip = client_get_random_intro(&service_kp.pubkey); tor_assert(!ip); }}}
The reason why I think it's okay to remove this is because
hs_cache_client_intro_state_find()andhs_cache_client_intro_state_note()would be callinghs_cache_client_intro_state_clean()which does it (and this test fails from the code I mentioned above if I did not delete it).So hmmm it appears that from the above,
hs_cache_client_intro_state_note()is used to mark the intro point as unusable and thus theclient_get_random_intro()can't find any usable intro.How is that related to the cleanup process that is about cache expiry? In other words, I'm not sure this test is suppose to fail all the sudden?
Trac:
Status: needs_review to needs_information Trac:
Cc: neel@neelc.org to N/A
Status: needs_information to assigned
Owner: neel to N/Amoved to tpo/core/tor#17520 (closed)
