[tor-bugs] #19720 [Metrics/CollecTor]: CollecTor should be re-configurable without restart

[Tor Bug Tracker & Wiki]

#19720: CollecTor should be re-configurable without restart
-------------------------------+---------------------------------
 Reporter:  iwakeh             |          Owner:  iwakeh
     Type:  enhancement        |         Status:  needs_review
 Priority:  High               |      Milestone:  CollecTor 1.0.0
Component:  Metrics/CollecTor  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:  ctip               |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:                     |        Sponsor:
-------------------------------+---------------------------------

Comment (by iwakeh):

 First some inline answers and then a summary below.

 Replying to [comment:8 karsten]:
 > Okay, I agree with most design decisions here, except for one that might
 confuse operators more than necessary: the modified time of the
 configuration file should not be checked once per minute (or even more
 often) but rather at the start of a new module run.  Imagine a situation
 where the next execution is less than 1 minute away and the operator makes
 a quick change to the configuration file.  There's no way for them to find
 out whether that change will make it into the next run or not, because the
 thread might check in 1 second or in 59 seconds.

 The Operator can answer this question from the log:

 Edit collector.properties and save.  The next check runs and reports the
 change:
 {{{
 2016-08-08 09:15:56,948 INFO o.t.c.c.Configuration:63 Configuration file
 was changed.
 }}}

 Modules starting after that will receive the new configuration, which is
 explicitly reported.
 {{{
 2016-08-08 09:16:57,026 INFO o.t.c.c.CollecTorMain:45 Module updateindex
 received new configuration.
 2016-08-08 09:16:57,027 INFO o.t.c.c.CollecTorMain:53 Starting updateindex
 module of CollecTor.
 }}}

 Of course, at the moment there is a possible waiting time of 60sec. See
 summary below.


 > Can we change that by taking out the thread and having modules check
 whether the configuration has changed?

 The modified config file would be read by each module. A potential error
 will be noticed very late and all modules would have to deal with it. And,
 all modules now have to keep track of when the configuration changed.

 > Ideally, we'd have a shell script that we use to reload the
 configuration file and possibly to verify it without loading it.  But
 that's for later.
 >

 That would be two steps:
 1. Edit and save.
 2. Don't forget to inform the running application (not necessarily using a
 shell script, but it could be a wrapper for that).

 Step 2 is easily forgotten especially if there is a stressful situation
 requiring the change.


 === Summary
 Maybe the following changes will help:

 a) Keep the centralized modified-check.
 b) Decrease check interval < 5 sec (is this fine for the impatient?).
 c) Every module could also log that it received the notice about a config
 change (in the `update` method).  Currently it logs only that it received
 the changed configuration before the run that uses it.

 Using a) the config change can be detected centrally and a change will be
 effective when saved w/o additional steps for the operator.
 Part b) will make the application's response more immediate. Maybe even
 log the entire configuration file in TRACE level.
 Part c) also gives early feedback that the various modules will use the
 new configuration in their next run.

 In addition to the existing logging this should be sufficient feedback to
 the operator.
 Here an example of the planned log entries:

 Edit collector.properties and save; the resulting midified time of the
 properties file is 09:15:52.  The next check runs a few seconds later and
 reports the change:
 {{{
 2016-08-08 09:15:56,948 INFO o.t.c.c.Configuration:63 Configuration file
 was changed.
 2016-08-08 09:15:56,949 INFO o.t.c.c.Configuration:63 Module updateindex
 will use new configuration.
 2016-08-08 09:15:56,949 INFO o.t.c.c.Configuration:63 Module relaydescs
 will use new configuration.
 ...
 2016-08-08 09:16:57,026 INFO o.t.c.c.CollecTorMain:45 Module updateindex
 received new configuration.
 2016-08-08 09:16:57,027 INFO o.t.c.c.CollecTorMain:53 Starting updateindex
 module of CollecTor.
 ...
 2016-08-08 10:09:00,001 INFO o.t.c.c.CollecTorMain:45 Module relaydescs
 received new configuration.
 2016-08-08 10:09:00,002 INFO o.t.c.c.CollecTorMain:53 Starting relaydescs
 module of CollecTor.
 }}}


 What do you think?

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