Skip to content
This repository has been archived by the owner. It is now read-only.

Maint 1.1 issue30727 measure known relays #361

Conversation

Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@juga0
Copy link
Contributor

@juga0 juga0 commented Jan 26, 2020

No description provided.

@juga0 juga0 force-pushed the maint-1.1_issue30727_measure_known_relays branch from cb41a8e to c79cdfa Jan 26, 2020
"""Whether the last consensus seen for this relay is older than the
measurement period.
"""
timestamp.is_old(self.last_consensus_timestamp)
Copy link
Member

@ahf ahf Jan 30, 2020

Shouldn't this "return" the value from is_old() ?

Copy link
Contributor Author

@juga0 juga0 Jan 31, 2020

auch, totally

@@ -415,28 +422,49 @@ def _init_relays(self):
# Change to stem.descriptor.remote in future refactor.
network_statuses = c.get_network_statuses()
new_relays_dict = dict([(r.fingerprint, r) for r in network_statuses])
log.debug("Number of relays in the current consensus: %s.",
len(new_relays_dict))
Copy link
Member

@ahf ahf Jan 30, 2020

%d instead of %s?

Copy link
Contributor Author

@juga0 juga0 Jan 31, 2020

hmm, it's python, i think it doesn't really matter. I changed, then changed the rest of logging lines where i'm using a number

# Only or debugging, count the relays that are not in the current
# consensus and have not been seen in the last consensuses either.
num_old_relays = 0

relays = copy.deepcopy(self._relays)
for r in relays:
if r.fingerprint in new_relays_dict.keys():
Copy link
Member

@ahf ahf Jan 30, 2020

I know this change is not for this patch, but one can do if key in dict without first generating a list of keys and check for an item in that list.

Copy link
Contributor Author

@juga0 juga0 Jan 31, 2020

hmm, we're generating 2 new things here: the dictionary to search for keys and an a copy of the list of relays. While the later isn't really needed and could just be modified, the former is useful to be able to easily search in a dictionary.
Ah, or you mean that it could be if r.fingerprint in new_relays_dict without the last .keys()

Copy link
Contributor

@teor2345 teor2345 Feb 3, 2020

It's much faster if you don't use .keys().

Searching a list takes a time that depends on the length of the list (and the position of the relay in the list). That could be a long time for a big list of relays.

Searching a dict usually takes a shorter amount of time, because it's a hash table.


# XXX: tech-debt: replace all the code that check whether a
# measurement or relay is older than the measurement period by this.
def is_old(timestamp, measurements_period=MEASUREMENTS_PERIOD):
Copy link
Member

@ahf ahf Jan 30, 2020

Good!

Keep the relays that are not in the the last consensus, but are not
"old" yet.

Closes: #30727
@juga0 juga0 force-pushed the maint-1.1_issue30727_measure_known_relays branch from c79cdfa to f01a6fe Jan 31, 2020
@torproject-pusher torproject-pusher merged commit f01a6fe into torproject:maint-1.1 Feb 5, 2020
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.