hs: Cache current time period number and SRV start time

For each node in the consensus, we call node_set_hsdir_index() which gets the time period and srv start time to learn if we are in between TP and SRV or not.

These calls ultimately call get_voting_schedule() which allocates memory and prints a debug log everytime.

All of this is actually pretty heavy and they are values we can cache because they are all based on the timings of a new consensus (valid_after, valid_until, fresh_until).

It is also called every second by a tor with a hidden service.

Caching those and updating when we get a new consensus seems a light refactoring for a good performance improvement.

changed milestone to %Tor: 0.3.2.x-final

added component::core tor/tor milestone::Tor: 0.3.2.x-final owner::dgoulet priority::medium prop224 resolution::implemented reviewer::asn severity::normal status::closed tor-hs tor-sr type::enhancement labels

Trac:
Keywords: N/A deleted, prop224 added

Trac:
Owner: N/A to dgoulet
Status: new to accepted

It was actually more straight forward than I thought.

Instead of keeping a star time of current round and time period and what not, we actually don't need to do such a thing but rather use the voting schedule static object instead. It is updated everytime we get a new consensus so using it is as close as we get to the latest.

Thus, now the SR subsystem only uses that object to compute it stuff.

Unit test is broken because we have an issue of timestamp being off because one function used to take a timestamp "now" but now it only uses the voting schedule timings so it needs to be fixed. However, I want to know if the approach seems sensible before going further.

See branch: ticket23623_032_01

Trac:
Status: accepted to needs_review
Reviewer: N/A to asn

Hmm, seems like we will only recompute voting schedule if we are a dirauth with this patch because of how dirvote_recalculate_timing() works.

We should probably change that, and document the new voting schedule setting behavior in static voting_schedule_t voting_schedule;.

Trac:
Status: needs_review to needs_revision

See extra commit in branch ticket23623_032_01.

I have still yet to fix the tests. Will get on that soon.

Ok tests fixed! That was a bit tricky. Everytime we mock a consensus, we have to recalculate its voting schedule for this kind of test now.

See branch: ticket23623_032_01

Trac:
Status: needs_revision to needs_review

LGTM. Did some tests connecting to hsv3s and it seems to work well.

Trac:
Status: needs_review to merge_ready

Why is the test_get_next_valid_after_time test removed? It's a little worrisome to remove unit tests. Do the same tests now get performed in some other way?

Before that patch, the function get_next_valid_after_time(time_t now), which was renamed in this patch to dirvote_get_next_valid_after_time(void), was allocating a voting schedule using now and returning the voting_schedule.interval_starts of that newly created schedule.

We now use a static voting schedule and update it with dirvote_recalculate_timing(), the test became pointless because that function now only returns interval_starts which is from the static voting schedule. Thus it is not needed as the test have been modified to use a static object and update it as it goes.

okay, that makes sense. I'm going to try merging this and looking at the test coverage.

Coverage looks fine. Merging forward and pushing!

Trac:
Status: merge_ready to closed
Resolution: N/A to implemented

ug, wait, I spoke too soon. Not merged yet -- could you please write a changes file?

Trac:
Status: closed to reopened
Resolution: implemented to N/A

Trac:
Status: reopened to needs_revision

Oops sorry about that. Pushed changes file in !ticket23623_032_01 in my repo.

Trac:
Status: needs_revision to merge_ready

Merged to 0.3.2 and forward!

Trac:
Status: merge_ready to closed
Resolution: N/A to implemented

closed

mentioned in issue #24161 (moved)

moved to tpo/core/tor#23623 (closed)

mentioned in issue tpo/core/tor#24161 (closed)