[tor-bugs] #22990 [Metrics/Library]: Add a heartbeat log message indicating progress and estimated time left

[Tor Bug Tracker & Wiki]

#22990: Add a heartbeat log message indicating progress and estimated time left
-----------------------------+--------------------------------
 Reporter:  karsten          |          Owner:  iwakeh
     Type:  enhancement      |         Status:  needs_revision
 Priority:  Medium           |      Milestone:
Component:  Metrics/Library  |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:  metrics-2017     |  Actual Points:
Parent ID:                   |         Points:
 Reviewer:  iwakeh           |        Sponsor:
-----------------------------+--------------------------------

Comment (by karsten):

 Replying to [comment:11 iwakeh]:
 > Replying to [comment:10 karsten]:
 > > ... Okay.
 >
 > Just some replies below to what we didn't totally agree on :-)
 >
 > >...
 > > > Information about skipped tarballs should also be logged to the
 'statistics' logger
 > >
 > > Not sure about that one. That's something different than what we're
 trying to achieve here: have an easy way to learn about progress when
 importing large amounts of data.
 >
 > Well, progress will be really fast, if tarballs are skipped.

 Ah, no, they won't be processed in the `readTarballs()` method at all,
 because they're not even added to `tarballs` in `readDescriptorFiles()`.

 > > > and the tarball skipped should be logged as warning as before.
 > >
 > > No, it's perfectly fine to leave a tarball in place and use the
 history file to avoid reading it once again. If we want to log it, it
 should be on debug.
 >
 > That will be an additional change.  Currently, these are logged as
 warning (I basically suggested it to keep the behavior we had):
 > [https://gitweb.torproject.org/user/karsten/metrics-
 lib.git/tree/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java?h=task-22990&id=663bdc285c4b5e0979691c44bf4790779df9bdf0#n245
 ` log.warn("Unable to read descriptor file {}.", file, e);` line 251]

 Oh, you mean skipped because of an `IOException`? Yes, we should keep that
 as a warning. I thought you meant skipped because we read them before.
 Alright, nothing to do here then.

 > > > Maybe, think about other possible statistics, which could be easily
 logged?
 > >
 > > Nothing came to mind yesterday. The new message was already very
 useful when importing one year of descriptors which took five hours. After
 three or four hours one wonders whether it's still doing anything useful
 or if it's going to take days or weeks, and then it's really useful to
 have this message. I'd say let's save future statistics for when we find
 out that we need them.
 >
 > Yep, that's fine.
 >
 > >
 > > I can revise the patch today or tomorrow, unless you beat me to it.
 >
 > Whoever starts on this should first add a tiny comment here :-)

 Sounds good!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22990#comment:13>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online