[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #30726 [Core Tor/Tor]: Missing relay keys in bandwidth file spec
#30726: Missing relay keys in bandwidth file spec
-------------------------------------+-----------------------------------
Reporter: teor | Owner: (none)
Type: defect | Status: needs_revision
Priority: High | Milestone: sbws: 1.1.x-final
Component: Core Tor/Tor | Version:
Severity: Major | Resolution:
Keywords: sbws-spec, sbws-roadmap | Actual Points:
Parent ID: #33121 | Points: 6
Reviewer: ahf, gk | Sponsor:
-------------------------------------+-----------------------------------
Changes (by gk):
* status: needs_review => needs_revision
Comment:
Replying to [comment:11 juga]:
> Replying to [comment:10 gk]:
>
> > > So i should check the collector to be sure about the which keys were
in which versions
> >
> > Sounds good.
>
> I checked and all the collected bandwidth files started with the
specification version 1.4.0, so in versions earlier than that it does not
really matter the KeyValues are not correct.
>
> Still, i fixed them in their respective versions, including the version
1.3.0 that was never generated by sbws (it should have been other ticket,
but the change is small).
>
> https://github.com/torproject/torspec/pull/110
Looks mostly good to me. I have some comments/nits mainly for
`0b6e97ca37b60fbfbd628306ec4b0551b34142cc`:
`4ec93f2d8b2add1da598d927785ed7d8e60f0ec3`: looks good
`0b6e97ca37b60fbfbd628306ec4b0551b34142cc`:
* `desc_bw_bur`, `consensus_bandwidth`, and
`consensus_bandwidth_is_unmeasured` are all in `v1.0.3` released which is
using `SPEC_VERSION` 1.2.0. Thus, those `KeyValues` are already in that
spec version available
* in the commit message s/They appeared in in/They appeared in/
* s/the consensus bandwdith/the consensus bandwidth/
* In the new "in the last data_period days" parts the default value for
"data_period days" is not mentioned anymore as it is in previous
`KeyValue` explanations. Maybe add a hint to the default one, here too
* In `relay_recent_priority_list_count` there is no "in the last
data_period days" but there should be, shouldn't it (at least that's what
`v3bwfile.py` is indicating)?
* "Add more KeyValues": I think it might make sense to use "Adds more
KeyValues" in the sense that a particular spec version *adds* those
things. I am a bit confused though, as the explanations to previous spec
versions alternate between "Add" and "Adds" without some pattern. You
could fix that part up in a follow-up commit if you think we should do
that.
`fb36a2bae2bc0aa1f0dfbfa68fbb903a5ee6d193`: looks good (modulo the
previously mentioned "Adds"/"Add" inconsistency; could be part of a
follow-up fix as well)
> What i should still check is whether we need to open tickets to metrics
and stem because of the change of version in the spec. I don't think so
because there were no bandwidth files published earlier than 1.4.0.
I agree that it should not be needed from my understanding but, yes, could
be worth double-checking, though.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30726#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs