Re: [tor-bugs] #21095 [Metrics/Onionoo]: Accept more values for the "order" parameter

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#21095: Accept more values for the "order" parameter
-----------------------------+------------------------------
 Reporter:  lukechilds       |          Owner:  metrics-team
     Type:  enhancement      |         Status:  needs_review
 Priority:  Medium           |      Milestone:
Component:  Metrics/Onionoo  |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:  metrics-help     |  Actual Points:
Parent ID:                   |         Points:
 Reviewer:                   |        Sponsor:
-----------------------------+------------------------------

Comment (by iwakeh):

 Replying to [comment:14 karsten]:
 > ...
 > > (Although they only test summary docs, but that's not really part of
 this ticket.)
 >
 > Right, though I wouldn't expect much better coverage from including
 other document types.  Details documents with the `fields` parameter
 maybe.  But all in all there are lower-hanging fruit for increasing actual
 test coverage.

 That's ok.

 >
 > > Maybe, add a test or change test data to accommodate testing the
 different order of order parameters. Currently `-first_seen` causes the
 same ordering of the test data as `consensus_weight`.
 >
 > Done in a new commit in the same branch.

 There needs to be a tie in the first order parameter in order to see the
 second being effective.
 Or is that the case in the test data?

 >
 > > The protocol
 [https://gitweb.torproject.org/user/karsten/onionoo.git/tree/src/main/resources/web/protocol.html?h=task-21095&id=f22b6626239a14f566d49081f55c5894fc5459a5#n476
 states] `Results are first ordered by the first list element, then by the
 second, and so on.`, but more than four order fields will cause the
 invalid request response `400`, which is tested for in
 `testOrderConsensusWeightFiveTimes` but not mentioned in the protocol.
 >
 > Right, I put that in to protect against potentially expensive queries
 asking us to sort results over and over.  What we could also do is specify
 that each parameter can only be used once.  Is that better?

 At most specifying each parameter once (either with or w/o the minus sign)
 is the optimal choice, I think.

 >
 > By the way, is there a valid use case for ordering by the same parameter
 more than once?  After all, the second (and subsequent) order parameters
 are only applied when two entries have the exact same value with respect
 to the first order parameter.

 There is no use-case for having two times the same parameter.

 >
 > Does the above become clear from the specification, or how would we
 explain that more clearly?  And is it what people would expect?

 Without changing code, the protocol should state in addition that more
 than four order parameters will cause an error; it states that the first
 of one type takes precedence.

 >
 > Example:
 >
 > {{{
 > [ {"a": 1, "b": 2, "c": 3},
 >   {"a": 1, "b": 1, "c": 4} ]
 >
 > ordered by a, b:
 > [ {"a": 1, "b": 1, "c": 4},
 >   {"a": 1, "b": 2, "c": 3} ]
 >
 > ordered by b, a:
 > [ {"a": 1, "b": 1, "c": 3},
 >   {"a": 1, "b": 2, "c": 4} ]
 > }}}

 {{{
 random order and maybe order by a:
  [ {"a": 1, "x": 2, "c": 3},
    {"a": 1, "x": 1, "c": 4} ]
 order a,x or just x:
  [ {"a": 1, "x": 1, "c": 4},
    {"a": 1, "x": 2, "c": 3} ]
 order a,c or just c:
  [ {"a": 1, "x": 2, "c": 3},
    {"a": 1, "x": 1, "c": 4} ]
 }}}

 But, I don't know if these examples might confuse rather than help.

 >
 > > How is the impact on memory when this new list is added?
 >
 > I didn't check, but in theory we're keeping another list in memory that
 contains 12k fingerprints times 40 bytes = 480 kB, plus some overhead for
 the list.  Not really an issue, I think.

 OK.

 >
 > > Unrelated: Should
 [https://gitweb.torproject.org/user/karsten/onionoo.git/tree/src/main/java/org/torproject/onionoo/server/NodeIndexer.java?h=task-21095&id=f22b6626239a14f566d49081f55c5894fc5459a5#n192
 this]:
 > >
 > > {{{
 > >     /* This variable can go away once all Onionoo services had their
 > >      * hourly updater write effective families to summary documents at
 > >      * least once.  Remove this code after September 8, 2015. */
 > > }}}
 > >
 > > be done now with an additional commit?
 >
 > Yes, let's do that in an additional commit, unrelated to this ticket.
 There maybe other parts in the code that we can take out.  But let's move
 that to another ticket.

 OK.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/21095#comment:15>
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