[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

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



#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 karsten):

 Replying to [comment:13 iwakeh]:
 > All fine and good to see all those tests :-)

 Great, thanks for looking!

 > (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.

 > 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.

 > 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?

 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.

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

 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} ]
 }}}

 > 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.

 > 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.

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